feat(sentry): redact bare rootkey-shaped tokens with an exact-shape base64-22 rule

backend/before-send.js

@@ -8,14 +8,12 @@
 // the same scrubbing + drop behaviour runs on Node-side events before
 // they leave the FGS.
 //
-// Mirrored from `src/sentry-scrub.ts`. The broad base64-22 token rule (to
-// catch bare rootKeys / public keys / project ids) is intentionally NOT
-// enabled here either — it over-matched Sentry's own 32-hex trace_ids,
-// PascalCase exception type names, and error_class metric tags, redacting
-// data we need. Pending a narrower design agreed with the team; bare tokens
-// are unscrubbed until then. Object fields keyed
-// lat/lng/latitude/longitude are redacted regardless of value type; lat/lng
-// markers redact the trailing number; HTTP breadcrumb URLs reduce to host-only.
+// Mirrored from `src/sentry-scrub.ts`. Bare (unmarked) rootkey-shaped
+// tokens are redacted by the exact-length base64-22 rule — see
+// BARE_KEY_TOKEN_PATTERN below for the design and its known limits.
+// Object fields keyed lat/lng/latitude/longitude are redacted regardless
+// of value type; lat/lng markers redact the trailing number; HTTP
+// breadcrumb URLs reduce to host-only.
 
 const REDACTED = "[redacted]";
 
@@ -29,6 +27,36 @@ const SCRUB_PATTERNS = [
   /\b(?:latitude|longitude|lat|lng|lon)\b\s*["']?\s*[:=]\s*-?\d+(?:\.\d+)?/gi,
 ];
 
+/**
+ * Bare rootkey-shaped token: exactly 22 base64/base64url chars whose last
+ * data char is [AQgw] (any valid 16-byte base64 ends there — its final 4
+ * bits are padding zeros), optional `==`, bounded by non-base64 chars.
+ * This exact-length + final-char anchor replaces the disabled "any
+ * 22+-char base64 run" rule that over-matched 32-hex trace_ids, PascalCase
+ * exception type names, and error_class metric tags. Matches still pass
+ * through isBareKeyShaped before redaction.
+ */
+const BARE_KEY_TOKEN_PATTERN =
+  /(?<![A-Za-z0-9+/=_-])[A-Za-z0-9+/_-]{21}[AQgw](?:==)?(?![A-Za-z0-9+/=_-])/g;
+
+/**
+ * Padded (`==`) matches are the exact rootkey wire shape — always redact.
+ * Unpadded ones must show a random-key character mix: both cases plus a
+ * digit/symbol (rejects PascalCase type names and ERR_* codes) and not
+ * pure hex (rejects truncated ids). Known limits: ~1% of random rootkeys
+ * are all-letters and slip an *unpadded* leak (the padded wire form still
+ * redacts), and a 22-char mixed-case identifier containing a digit that
+ * happens to end in [AQgw] is falsely redacted — an accepted trade-off,
+ * false negatives being worse here.
+ *
+ * @param {string} token
+ */
+function isBareKeyShaped(token) {
+  if (token.endsWith("==")) return true;
+  if (/^[0-9a-f]+$/i.test(token)) return false;
+  return /[a-z]/.test(token) && /[A-Z]/.test(token) && /[0-9+/_-]/.test(token);
+}
+
 /** Object keys whose value is a raw coordinate — redacted regardless of type. */
 const SENSITIVE_KEY_PATTERN = /^(lat|lng|lon|latitude|longitude)$/i;
 
@@ -48,18 +76,30 @@ const FORBIDDEN_METRIC_TAG_NAMES = new Set([
   "rootkey",
 ]);
 
-/** @type {RegExp[]} */
+/** Forbidden tag *values* — lat/lng shapes. (Bare rootkey-shaped values
+ *  are checked separately via containsBareKeyToken.)
+ *  @type {RegExp[]} */
 const FORBIDDEN_METRIC_VALUE_PATTERNS = [
   /\b(?:latitude|longitude|lat|lng|lon)\b\s*["']?\s*[:=]\s*-?\d+(?:\.\d+)?/i,
 ];
 
+/** @param {string} value */
+function containsBareKeyToken(value) {
+  for (const match of value.matchAll(BARE_KEY_TOKEN_PATTERN)) {
+    if (isBareKeyShaped(match[0])) return true;
+  }
+  return false;
+}
+
 /** @param {string} input */
 export function scrubString(input) {
   let out = input;
   for (const pattern of SCRUB_PATTERNS) {
     out = out.replace(pattern, REDACTED);
   }
-  return out;
+  return out.replace(BARE_KEY_TOKEN_PATTERN, (match) =>
+    isBareKeyShaped(match) ? REDACTED : match,
+  );
 }
 
 /** Reduce an HTTP(S) URL to scheme + host. @param {string} url */
@@ -241,6 +281,7 @@ export function isForbiddenMetric(name, attributes) {
       for (const pattern of FORBIDDEN_METRIC_VALUE_PATTERNS) {
         if (pattern.test(tagValue)) return true;
       }
+      if (containsBareKeyToken(tagValue)) return true;
     }
   }
   return false;

backend/lib/metrics.test.mjs

@@ -145,8 +145,10 @@ test("before_metric_send drops forbidden tag VALUES (lat/lng shape)", () => {
     0,
     "metric carrying a lat/lng tag value must be dropped",
   );
-  // A normal bucket value passes. (The broad base64-22 value rule is disabled
-  // pending a narrower design — a bare token would also pass now.)
+  // A bare rootkey-shaped value is also dropped.
+  metrics.storageSizeBucket("bm90LWEtcmVhbC1rZXktMQ");
+  assert.equal(calls.count.length, 0);
+  // A normal bucket value passes.
   metrics.storageSizeBucket("<10MB");
   assert.equal(calls.count.length, 1);
 });

docs/sentry-integration.md

@@ -1287,12 +1287,29 @@ Node; shared test cases in `test-support/scrubber-cases.js` keep the
 two copies from drifting): it walks the event tree — message, exception
 values, extra, contexts, breadcrumbs, structured logs — redacting
 `rootKey`-marked values and coordinate markers (`lat`, `lng`, `lon`,
-`latitude`, `longitude`, in key/value and JSON-serialized forms). A
-broad base64-22-char rule (to catch *bare* unmarked rootkeys) is
-deliberately **not** enabled: it also matched trace IDs and exception
-type names; a narrower design is pending, and until then bare unmarked
-tokens pass through. This is belt-and-suspenders — the fix is always at
-the capture site, but the scrubber catches mistakes before they ship.
+`latitude`, `longitude`, in key/value and JSON-serialized forms).
+
+*Bare* (unmarked) rootkey-shaped tokens are caught by an exact-shape
+rule rather than the originally proposed "any 22+-char base64 run"
+(which over-matched 32-hex trace IDs, PascalCase exception type names,
+and `error_class` metric tags). The rootkey crosses the wire as strict
+base64 of 16 bytes — `backend/index.js` enforces
+`/^[A-Za-z0-9+/]{22}==$/` — and any valid 16-byte base64 ends its 22nd
+char in `A`/`Q`/`g`/`w` (the final 4 bits are padding zeros). So the
+rule matches exactly 22 base64/base64url chars ending in `[AQgw]`,
+optionally `==`-padded, bounded by non-base64 chars; a padded match is
+the exact wire shape and always redacts, while an unpadded match must
+also show a random-key character mix (both cases plus a digit or
+symbol, and not pure hex). That structurally spares trace/span IDs
+(wrong length, hex), type names and `ERR_*` codes (letters-only or
+single-case), and base64-ish IDs of other lengths (43-char public keys,
+52-char project IDs). Known limits: roughly 1% of random rootkeys are
+all-letters and would slip through in *unpadded* form (the padded wire
+form still redacts), and a 22-char mixed-case identifier containing a
+digit that happens to end in `[AQgw]` is falsely redacted — accepted,
+since false negatives are worse here. This is belt-and-suspenders — the
+fix is always at the capture site, but the scrubber catches mistakes
+before they ship.
 
 ---
 

src/__tests__/sentry-metrics.test.js

@@ -93,11 +93,11 @@ describe("sentry-metrics", () => {
     expect(calls).toHaveLength(1);
   });
 
-  // The broad base64-22 value rule is disabled pending a narrower design
-  // (see sentry-scrub.ts), so a bare token tag no longer drops the metric.
-  test("bare base64 tag values pass through while the broad rule is disabled", () => {
+  test("bare rootkey-shaped tag values drop the metric; error_class survives", () => {
     const { __metricsInternals } = require("../sentry-metrics");
     __metricsInternals.count("comapeo.x", { bucket: "bm90LWEtcmVhbC1rZXktMQ" });
+    expect(calls).toHaveLength(0);
+    __metricsInternals.count("comapeo.rpc.errors", { error_class: "NotFoundError" });
     expect(calls).toHaveLength(1);
   });
 

src/__tests__/sentry.test.js

@@ -310,8 +310,7 @@ describe("initSentry", () => {
     expect(payload).toContain("[redacted]");
     expect(payload).not.toContain("aGVsbG8td29ybGQtMTIzNA"); // rootKey value gone
     expect(payload).not.toContain("-12.34"); // lat/lng gone
-    // Broad base64-22 rule disabled: a bare token in `extra` currently survives.
-    expect(payload).toContain("bm90LWEtcmVhbC1rZXktMQ");
+    expect(payload).not.toContain("bm90LWEtcmVhbC1rZXktMQ"); // bare rootkey-shaped token gone
   });
 
   test("beforeBreadcrumb reduces HTTP URLs to host-only", () => {

src/sentry-scrub.ts

@@ -11,10 +11,13 @@
  * buggy host (and our own mistakes) before a payload leaves the device.
  *
  * What it redacts (and the false-positive trade-off):
- *   - Explicit `rootKey=…` markers (key=value / json / prose). A broad
- *     "any 22+-char base64url run" rule is deliberately NOT enabled — see
- *     the SCRUB_PATTERNS note below — so bare rootKeys/keys/project-ids
- *     with no marker are currently unscrubbed.
+ *   - Explicit `rootKey=…` markers (key=value / json / prose).
+ *   - Bare (unmarked) rootkey-shaped tokens: exactly 22 base64 chars
+ *     ending in [AQgw] — the only shape a 16-byte base64 rootkey can take
+ *     (`backend/index.js` enforces /^[A-Za-z0-9+/]{22}==$/ on the wire).
+ *     Padded (`==`) matches always redact; unpadded ones also need a
+ *     character-mix check — see BARE_KEY_TOKEN_PATTERN below for the rule
+ *     and its known limits.
  *   - Object fields whose KEY is lat/lng/latitude/longitude are redacted
  *     regardless of value type — a numeric `{latitude: 12.3}` is the most
  *     likely capture shape and value-only scrubbing would miss it.
@@ -39,18 +42,40 @@ const SCRUB_PATTERNS: RegExp[] = [
   // field delimiter (whitespace, `,;&`, quote) so co-located fields in a
   // compact string like `rootKey=abc,method=x` survive.
   /\broot[_-]?key\b\s*["']?\s*[:=]\s*[^\s,;&"']+/gi,
-  // NOTE: a broad 22+-char URL-safe base64 rule (to catch bare rootKeys at
-  // 22, public keys at 43, project ids at ~52) is intentionally NOT enabled
-  // — it also matched Sentry's own 32-hex trace_ids, PascalCase exception
-  // type names, and error_class metric tags, redacting data we need. Pending
-  // a narrower design agreed with the team; bare tokens are unscrubbed until
-  // then.
   // Latitude / longitude markers followed by a number. `lon` is the field
   // name @comapeo/schema observations actually use. Optional quote between
   // key and separator so JSON-serialized coordinates (`"lat":-12.3`) match.
   /\b(?:latitude|longitude|lat|lng|lon)\b\s*["']?\s*[:=]\s*-?\d+(?:\.\d+)?/gi,
 ];
 
+/**
+ * Bare rootkey-shaped token: exactly 22 base64/base64url chars whose last
+ * data char is [AQgw] (any valid 16-byte base64 ends there — its final 4
+ * bits are padding zeros), optional `==`, bounded by non-base64 chars.
+ * This exact-length + final-char anchor replaces the disabled "any
+ * 22+-char base64 run" rule that over-matched 32-hex trace_ids, PascalCase
+ * exception type names, and error_class metric tags. Matches still pass
+ * through isBareKeyShaped before redaction.
+ */
+const BARE_KEY_TOKEN_PATTERN =
+  /(?<![A-Za-z0-9+/=_-])[A-Za-z0-9+/_-]{21}[AQgw](?:==)?(?![A-Za-z0-9+/=_-])/g;
+
+/**
+ * Padded (`==`) matches are the exact rootkey wire shape — always redact.
+ * Unpadded ones must show a random-key character mix: both cases plus a
+ * digit/symbol (rejects PascalCase type names and ERR_* codes) and not
+ * pure hex (rejects truncated ids). Known limits: ~1% of random rootkeys
+ * are all-letters and slip an *unpadded* leak (the padded wire form still
+ * redacts), and a 22-char mixed-case identifier containing a digit that
+ * happens to end in [AQgw] is falsely redacted — an accepted trade-off,
+ * false negatives being worse here.
+ */
+function isBareKeyShaped(token: string): boolean {
+  if (token.endsWith("==")) return true;
+  if (/^[0-9a-f]+$/i.test(token)) return false;
+  return /[a-z]/.test(token) && /[A-Z]/.test(token) && /[0-9+/_-]/.test(token);
+}
+
 /** Object keys whose value is a raw coordinate — redacted regardless of type. */
 const SENSITIVE_KEY_PATTERN = /^(lat|lng|lon|latitude|longitude)$/i;
 
@@ -71,18 +96,27 @@ const FORBIDDEN_METRIC_TAG_NAMES = new Set([
   "rootkey",
 ]);
 
-/** Forbidden tag *values* — lat/lng shapes. (The broad base64-22 rule is
- *  held back here too; see the SCRUB_PATTERNS note above.) */
+/** Forbidden tag *values* — lat/lng shapes. (Bare rootkey-shaped values
+ *  are checked separately via containsBareKeyToken.) */
 const FORBIDDEN_METRIC_VALUE_PATTERNS: RegExp[] = [
   /\b(?:latitude|longitude|lat|lng|lon)\b\s*["']?\s*[:=]\s*-?\d+(?:\.\d+)?/i,
 ];
 
+function containsBareKeyToken(value: string): boolean {
+  for (const match of value.matchAll(BARE_KEY_TOKEN_PATTERN)) {
+    if (isBareKeyShaped(match[0])) return true;
+  }
+  return false;
+}
+
 export function scrubString(input: string): string {
   let out = input;
   for (const pattern of SCRUB_PATTERNS) {
     out = out.replace(pattern, REDACTED);
   }
-  return out;
+  return out.replace(BARE_KEY_TOKEN_PATTERN, (match) =>
+    isBareKeyShaped(match) ? REDACTED : match,
+  );
 }
 
 /** Reduce an HTTP(S) URL to scheme + host, dropping path/query/fragment. */
@@ -261,6 +295,7 @@ export function isForbiddenMetric(
       for (const pattern of FORBIDDEN_METRIC_VALUE_PATTERNS) {
         if (pattern.test(tagValue)) return true;
       }
+      if (containsBareKeyToken(tagValue)) return true;
     }
   }
   return false;

test-support/scrubber-cases.js

@@ -11,6 +11,9 @@ export const BASE64_43 = "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8";
 // ~52-char z-base-32 project id.
 export const ZBASE32_52 =
   "ybybybybybybybybybybybybybybybybybybybybybybybybybyb";
+// 16-byte rootkey in the wire format `backend/index.js` enforces:
+// 22 base64 chars + `==` (base64 of the ASCII bytes "0123456789abcdef").
+export const ROOTKEY_PADDED = "MDEyMzQ1Njc4OWFiY2RlZg==";
 
 /** `scrubString(input)` must equal `expect`. */
 export const scrubStringCases = [
@@ -26,8 +29,21 @@ export const scrubStringCases = [
   // co-located fields in a compact string survive.
   { name: "rootKey value stops at comma delimiter", input: "rootKey=abc,method=obs.create,code=500", expect: "[redacted],method=obs.create,code=500" },
   { name: "plain sentence untouched", input: "hello world", expect: "hello world" },
-  // Broad base64-22 rule is intentionally disabled — bare tokens pass through.
-  { name: "bare base64 token passes through", input: "token bm90LWEtcmVhbC1rZXktMQ done", expect: "token bm90LWEtcmVhbC1rZXktMQ done" },
+  // Bare (unmarked) rootkey-shaped tokens: exact-length base64-22 rule.
+  { name: "bare padded rootkey (wire format) redacted", input: `boot got key ${ROOTKEY_PADDED} from native`, expect: "boot got key [redacted] from native" },
+  { name: "bare unpadded rootkey-shaped token redacted", input: "token bm90LWEtcmVhbC1rZXktMQ done", expect: "token [redacted] done" },
+  // The marker rule can't reach a JSON-quoted value (quote delimits it);
+  // the bare rule catches the token inside the quotes.
+  { name: "JSON-quoted rootkey value redacted by bare rule", input: `{"rootKey":"${ROOTKEY_PADDED}"}`, expect: '{"rootKey":"[redacted]"}' },
+  // Shapes the disabled broad rule over-matched — must all survive.
+  { name: "32-hex trace id and 16-hex span id survive", input: "trace 4bf92f3577b34da6a3ce929d0e0e4736 span 00f067aa0ba902b7", expect: "trace 4bf92f3577b34da6a3ce929d0e0e4736 span 00f067aa0ba902b7" },
+  { name: "exception type names survive", input: "NotFoundError: caused by TypeError", expect: "NotFoundError: caused by TypeError" },
+  // 22 letters ending in [AQgw] — only the character-mix check spares it.
+  { name: "22-char PascalCase identifier survives", input: "handled ProjectInviteTokenView event", expect: "handled ProjectInviteTokenView event" },
+  // 22 chars, right final char, mixed case + digits — the hex check spares it.
+  { name: "22-char hex fragment survives", input: "id deadbeefDEADBEEF0011AA end", expect: "id deadbeefDEADBEEF0011AA end" },
+  { name: "22-char ERR_* code survives", input: "ERR_INVALID_URL_SCHEME", expect: "ERR_INVALID_URL_SCHEME" },
+  // Exact-length rule ignores base64-ish tokens of other lengths.
   { name: "43-char public key passes through", input: BASE64_43, expect: BASE64_43 },
   { name: "52-char project id passes through", input: ZBASE32_52, expect: ZBASE32_52 },
 ];
@@ -49,8 +65,11 @@ export const forbiddenMetricCases = [
   { name: "forbidden metric name", metricName: "project_id", attributes: { platform: "ios" }, expect: true },
   { name: "lat/lng-shaped tag value", metricName: "comapeo.x", attributes: { coord: "lat=12.34" }, expect: true },
   { name: "lon-shaped tag value", metricName: "comapeo.x", attributes: { coord: "lon=-55.12" }, expect: true },
-  // Broad base64-22 value rule disabled — bare tokens no longer drop a metric.
-  { name: "43-char token value allowed (broad rule off)", metricName: "comapeo.x", attributes: { bucket: BASE64_43 }, expect: false },
-  { name: "52-char token value allowed (broad rule off)", metricName: "comapeo.x", attributes: { bucket: ZBASE32_52 }, expect: false },
+  { name: "bare rootkey-shaped tag value drops the metric", metricName: "comapeo.x", attributes: { note: `key ${ROOTKEY_PADDED}` }, expect: true },
+  { name: "error_class tag value allowed", metricName: "comapeo.rpc.errors", attributes: { error_class: "NotFoundError" }, expect: false },
+  { name: "trace-id tag value allowed", metricName: "comapeo.x", attributes: { trace: "4bf92f3577b34da6a3ce929d0e0e4736" }, expect: false },
+  // Exact-length rule ignores base64-ish tokens of other lengths.
+  { name: "43-char token value allowed", metricName: "comapeo.x", attributes: { bucket: BASE64_43 }, expect: false },
+  { name: "52-char token value allowed", metricName: "comapeo.x", attributes: { bucket: ZBASE32_52 }, expect: false },
   { name: "ordinary rpc tags allowed", metricName: "comapeo.rpc.client.duration_ms", attributes: { method: "read.doc", status: "ok", platform: "ios" }, expect: false },
 ];