Repository: honeycombio/beeline-nodejs
Pull request #241 (2020-08-05 23:12:30)

validation improvements in w3c
Per discussion earlier today, bringing this beeline in line with the w3c specifications for where we should have opinions on id validation. Most importantly, we shouldn't propagate any w3c headers that don't meet w3c specification. w3c validation boils down to character count and allowed characters; we agreed to use regex for this, similar to opentelemetry.

Since we're using the opentelemetry parse function, I'm leaving the parse version handling there (using a regex in ruby for this).

Spec for those curious: https://www.w3.org/TR/trace-context/#versioning-of-traceparent

katiebayes: Re-requesting (sorry), just want to make sure we're good on the debug usage here
katiebayes: Confirmed via slack, we're good here!

Patch:
diff --git a/lib/propagation/w3c.js b/lib/propagation/w3c.js
index 7b8589c..3c2a680 100644
--- a/lib/propagation/w3c.js
+++ b/lib/propagation/w3c.js
@@ -1,5 +1,8 @@
-/* global require, exports */
+/* global require, exports, __dirname */
 const { TRACE_PARENT_HEADER, parseTraceParent } = require("@opentelemetry/core"),
+  path = require("path"),
+  pkg = require(path.join(__dirname, "..", "..", "package.json")),
+  debug = require("debug")(`${pkg.name}:propagation`),
   util = require("./util");
 // requiring the OTEL trace header key from the api
 // as well as the parser
@@ -8,6 +11,9 @@ exports.TRACE_HTTP_HEADER = TRACE_PARENT_HEADER;
 const VERSION = "00";
 exports.VERSION = VERSION;
 
+const TRACE_ID_REGEX = /^[A-Fa-f0-9]{32}$/g;
+const SPAN_ID_REGEX = /^[A-Fa-f0-9]{16}$/g;
+
 exports.unmarshalTraceContext = unmarshalTraceContext;
 
 // unmarshalTraceContext takes an incoming header
@@ -59,6 +65,19 @@ function marshalTraceContext(context) {
 function marshalTraceContextv1(context) {
   const spanId = util.currentSpanId(context);
 
+  // do not propagate non-standard ids
+  if (!context.id.match(TRACE_ID_REGEX)) {
+    debug(
+      "unable to propagate w3c trace header: trace id must be a 32-character hex encoded string"
+    );
+    return "";
+  }
+
+  if (!spanId.match(SPAN_ID_REGEX)) {
+    debug("unable to propagate w3c span header: span id must be a 16-character hex encoded string");
+    return "";
+  }
+
   // currently we do not propagate traceFlags
   // we will revisit as opentelemetry sampling evolves
   return `${VERSION}-${context.id}-${spanId}-01`;


Test patch:
diff --git a/lib/propagation/propagation.test.js b/lib/propagation/propagation.test.js
index aa38009..1e9acde 100644
--- a/lib/propagation/propagation.test.js
+++ b/lib/propagation/propagation.test.js
@@ -143,6 +143,26 @@ cases(
       },
       value: "00-7f042f75651d9782dcff93a45fa99be0-c998e73e5420f609-01",
     },
+    {
+      name: "do not propagate headers if trace id is non-standard",
+      context: {
+        id: "7f042f75651d9782dc",
+        dataset: "testDataset",
+        stack: [new Span({ [schema.TRACE_SPAN_ID]: "c998e73e5420f609" })],
+        traceContext: {},
+      },
+      value: "",
+    },
+    {
+      name: "do not propagate headers if span id is non-standard",
+      context: {
+        id: "7f042f75651d9782dcff93a45fa99be0",
+        dataset: "testDataset",
+        stack: [new Span({ [schema.TRACE_SPAN_ID]: "c998e73e542*f609358" })],
+        traceContext: {},
+      },
+      value: "",
+    },
     {
       name: "honeycomb root",
       context: {