Drive marker PII sanitization from schemas

docs-developer/CHANGELOG-formats.md

@@ -6,6 +6,10 @@ Note that this is not an exhaustive list. Processed profile format upgraders can
 
 ## Processed profile format
 
+### Version 72
+
+Marker schema fields can now include a `containsPII` array describing the categories of privacy-sensitive data they contain. Profile sanitization uses these categories instead of identifying privacy-sensitive fields from the marker type.
+
 ### Version 71
 
 The frame table (`profile.shared.frameTable`) representation changed in such a way that all its columns can now be typed arrays when using [JsonSlabs](https://github.com/mstange/json-slabs/) profiles.

src/app-logic/constants.ts

@@ -12,7 +12,7 @@ export const GECKO_PROFILE_VERSION = 36;
 // The current version of the "processed" profile format.
 // Please don't forget to update the processed profile format changelog in
 // `docs-developer/CHANGELOG-formats.md`.
-export const PROCESSED_PROFILE_VERSION = 71;
+export const PROCESSED_PROFILE_VERSION = 72;
 
 // The following are the margin sizes for the left and right of the timeline. Independent
 // components need to share these values.

src/profile-logic/marker-data.ts

@@ -23,6 +23,7 @@ import {
 } from 'firefox-profiler/app-logic/constants';
 import {
   getSchemaFromMarker,
+  isStringIndexFormat,
   isStringIndexMarkerField,
   markerPayloadMatchesSearch,
   markerSchemaFrontEndOnly,
@@ -43,18 +44,18 @@ import type {
   IPCSharedData,
   IPCMarkerPayload,
   NetworkPayload,
-  PrefMarkerPayload,
-  TextMarkerPayload,
   StartEndRange,
   IndexedArray,
   DerivedMarkerInfo,
   MarkerSchema,
+  MarkerSchemaPIICategory,
   MarkerSchemaByName,
   MarkerDisplayLocation,
   Tid,
   LogMarkerPayload,
   ThreadIndex,
   Profile,
+  RemoveProfileInformation,
 } from 'firefox-profiler/types';
 
 /**
@@ -1456,132 +1457,153 @@ export function groupScreenshotsById(
   return idToScreenshotMarkers;
 }
 
-export function removeNetworkMarkerURLs(
-  payload: NetworkPayload
-): NetworkPayload {
-  return { ...payload, URI: '', RedirectURI: '' };
+function _removeExtensionId(markerName: string, text: string): string {
+  if (['ExtensionParent', 'ExtensionChild'].includes(markerName)) {
+    return text.replace(/^.*, (api_(call|event): )/, '$1');
+  }
+
+  if (markerName === 'Extension Suspend') {
+    return text.replace(/ by .*$/, '');
+  }
+
+  return text;
 }
 
-export function removePrefMarkerPreferenceValues(
-  payload: PrefMarkerPayload
-): PrefMarkerPayload {
-  return { ...payload, prefValue: '' };
+function _shouldSanitizePIICategory(
+  category: MarkerSchemaPIICategory,
+  PIIToBeRemoved: RemoveProfileInformation
+): boolean {
+  switch (category) {
+    case 'url':
+      return PIIToBeRemoved.shouldRemoveUrls;
+    case 'extension-id':
+      return PIIToBeRemoved.shouldRemoveExtensions;
+    case 'preference-value':
+      return PIIToBeRemoved.shouldRemovePreferenceValues;
+    case 'private-browsing':
+      return PIIToBeRemoved.shouldRemovePrivateBrowsingData;
+    default:
+      assertExhaustiveCheck(category);
+      return false;
+  }
 }
 
-/**
- * Apply a transformation to a Text marker's text. The schema tells us whether the
- * payload holds the text inline or as a string table index. In the latter case the
- * result is interned as a new string, as other markers and frames may share that
- * entry.
- */
-function _updateTextMarkerText(
-  payload: TextMarkerPayload,
-  stringIndexMarkerFieldsByDataType: Map<string, string[]>,
+function _updateMarkerPayloadField(
+  markerPayload: MarkerPayload,
+  key: string,
+  isStringIndex: boolean,
   stringTable: StringTable,
   transform: (text: string) => string
-): TextMarkerPayload {
-  // The casts below follow the storage layout the schema declares, which
-  // TypeScript can't verify from the payload type alone.
-  if (
-    !isStringIndexMarkerField(
-      stringIndexMarkerFieldsByDataType,
-      payload.type,
-      'name'
-    )
-  ) {
-    return { ...payload, name: transform(payload.name as string) };
+): MarkerPayload {
+  const value = (markerPayload as any)[key];
+  if (!isStringIndex) {
+    return { ...markerPayload, [key]: transform(value) } as any;
   }
 
-  const nameIndex = payload.name as IndexIntoStringTable;
-  if (!stringTable.hasIndex(nameIndex)) {
-    return payload;
+  const stringIndex = value as IndexIntoStringTable;
+  if (!stringTable.hasIndex(stringIndex)) {
+    return markerPayload;
   }
-  const text = stringTable.getString(nameIndex);
+  const text = stringTable.getString(stringIndex);
   const newText = transform(text);
   if (newText === text) {
-    return payload;
-  }
-  return { ...payload, name: stringTable.indexForString(newText) };
-}
-
-/**
- * Sanitize Text marker's name property for potential URLs. Only for payloads
- * holding their text inline, as the string table is sanitized as a whole.
- */
-export function sanitizeTextMarker(
-  payload: TextMarkerPayload,
-  stringIndexMarkerFieldsByDataType: Map<string, string[]>,
-  stringTable: StringTable
-): TextMarkerPayload {
-  return _updateTextMarkerText(
-    payload,
-    stringIndexMarkerFieldsByDataType,
-    stringTable,
-    removeURLs
-  );
-}
-
-/**
- * Sanitize Extension Text marker's name property for potential add-on ids.
- */
-export function sanitizeExtensionTextMarker(
-  markerName: string,
-  payload: TextMarkerPayload,
-  stringIndexMarkerFieldsByDataType: Map<string, string[]>,
-  stringTable: StringTable
-): TextMarkerPayload {
-  if (['ExtensionParent', 'ExtensionChild'].includes(markerName)) {
-    return _updateTextMarkerText(
-      payload,
-      stringIndexMarkerFieldsByDataType,
-      stringTable,
-      (text) => text.replace(/^.*, (api_(call|event): )/, '$1')
-    );
+    return markerPayload;
   }
-
-  if (markerName === 'Extension Suspend') {
-    return _updateTextMarkerText(
-      payload,
-      stringIndexMarkerFieldsByDataType,
-      stringTable,
-      (text) => text.replace(/ by .*$/, '')
-    );
-  }
-
-  return payload;
+  return {
+    ...markerPayload,
+    [key]: stringTable.indexForString(newText),
+  } as any;
 }
 
-export function sanitizeFromMarkerSchema(
+/** Apply a marker schema's PII rules to its payload. */
+export function sanitizeMarkerFromSchema(
   markerSchema: MarkerSchema,
-  markerPayload: MarkerPayload
-): MarkerPayload {
-  for (const { key, format } of markerSchema.fields) {
-    if (!(key in markerPayload)) {
-      continue;
+  markerName: string,
+  markerPayload: MarkerPayload,
+  stringTable: StringTable,
+  PIIToBeRemoved: RemoveProfileInformation
+): {
+  markerPayload: MarkerPayload;
+  shouldRemoveMarker: boolean;
+} {
+  let shouldRemoveMarker = false;
+
+  for (const { key, format, containsPII = [] } of markerSchema.fields) {
+    const hasField = key in markerPayload;
+
+    if (hasField) {
+      // The casts are needed because TypeScript cannot refine the payload union
+      // using a schema field that is only known at runtime.
+      if (PIIToBeRemoved.shouldRemoveUrls && format === 'url') {
+        markerPayload = {
+          ...markerPayload,
+          [key]: removeURLs((markerPayload as any)[key]),
+        } as any;
+      } else if (PIIToBeRemoved.shouldRemoveUrls && format === 'file-path') {
+        markerPayload = {
+          ...markerPayload,
+          [key]: removeFilePath((markerPayload as any)[key]),
+        } as any;
+      } else if (
+        PIIToBeRemoved.shouldRemoveUrls &&
+        format === 'sanitized-string'
+      ) {
+        markerPayload = {
+          ...markerPayload,
+          [key]: '<sanitized>',
+        } as any;
+      }
     }
 
-    // We're typing the result of the sanitization with `any` because Flow
-    // doesn't like much our enormous enum of non-exact objects that's used as
-    // MarkerPayload type, and this code is too generic for Flow in this context.
-    if (format === 'url') {
-      markerPayload = {
-        ...markerPayload,
-        [key]: removeURLs((markerPayload as any)[key]),
-      } as any;
-    } else if (format === 'file-path') {
-      markerPayload = {
-        ...markerPayload,
-        [key]: removeFilePath((markerPayload as any)[key]),
-      } as any;
-    } else if (format === 'sanitized-string') {
-      markerPayload = {
-        ...markerPayload,
-        [key]: '<sanitized>',
-      } as any;
+    const isStringIndex = isStringIndexFormat(format);
+    for (const category of containsPII) {
+      if (!_shouldSanitizePIICategory(category, PIIToBeRemoved)) {
+        continue;
+      }
+
+      switch (category) {
+        case 'url':
+          if (hasField) {
+            markerPayload = _updateMarkerPayloadField(
+              markerPayload,
+              key,
+              isStringIndex,
+              stringTable,
+              removeURLs
+            );
+          }
+          break;
+        case 'extension-id':
+          if (hasField) {
+            markerPayload = _updateMarkerPayloadField(
+              markerPayload,
+              key,
+              isStringIndex,
+              stringTable,
+              (text) => _removeExtensionId(markerName, text)
+            );
+          }
+          break;
+        case 'preference-value':
+          markerPayload = _updateMarkerPayloadField(
+            markerPayload,
+            key,
+            isStringIndex,
+            stringTable,
+            () => ''
+          );
+          break;
+        case 'private-browsing':
+          shouldRemoveMarker ||=
+            hasField && Boolean((markerPayload as any)[key]);
+          break;
+        default:
+          assertExhaustiveCheck(category);
+      }
     }
   }
 
-  return markerPayload;
+  return { markerPayload, shouldRemoveMarker };
 }
 
 /**

src/profile-logic/marker-schema.ts

@@ -21,6 +21,7 @@ import type {
   MarkerSchema,
   MarkerSchemaByName,
   MarkerSchemaField,
+  MarkerSchemaPIICategory,
   Marker,
   MarkerIndex,
   MarkerPayload,
@@ -29,12 +30,61 @@ import type {
 } from 'firefox-profiler/types';
 import type { StringTable } from '../utils/string-table';
 
+// Profiles recorded by Gecko versions without PII annotations use these defaults.
+const markerSchemaPIICategoriesBySchemaName = new Map<
+  string,
+  Map<string, MarkerSchemaPIICategory[]>
+>([
+  [
+    'Network',
+    new Map([
+      ['URI', ['url']],
+      ['RedirectURI', ['url']],
+      ['isPrivateBrowsing', ['private-browsing']],
+    ]),
+  ],
+  ['Text', new Map([['name', ['url', 'extension-id']]])],
+  ['PreferenceRead', new Map([['prefValue', ['preference-value']]])],
+]);
+
+export function addPIICategoriesToMarkerSchema(
+  markerSchema: MarkerSchema
+): MarkerSchema {
+  const piiCategoriesByField = markerSchemaPIICategoriesBySchemaName.get(
+    markerSchema.name
+  );
+  if (!piiCategoriesByField) {
+    return markerSchema;
+  }
+
+  const fields = markerSchema.fields.map((field) => {
+    const containsPII = piiCategoriesByField.get(field.key);
+    return containsPII && !field.containsPII
+      ? { ...field, containsPII }
+      : field;
+  });
+  const existingFieldKeys = new Set(fields.map(({ key }) => key));
+  for (const [key, containsPII] of piiCategoriesByField) {
+    if (!existingFieldKeys.has(key)) {
+      fields.push({ key, format: 'string', hidden: true, containsPII });
+    }
+  }
+
+  return { ...markerSchema, fields };
+}
+
+export function addPIICategoriesToMarkerSchemas(
+  markerSchemas: MarkerSchema[]
+): MarkerSchema[] {
+  return markerSchemas.map(addPIICategoriesToMarkerSchema);
+}
+
 /**
  * The marker schema comes from Gecko, and is embedded in the profile. However,
  * we may want to define schemas that are front-end only. This is the location
  * to do that. The schema will get merged in with the Gecko schema.
  */
-export const markerSchemaFrontEndOnly: MarkerSchema[] = [
+const markerSchemaFrontEndOnlyWithoutPII: MarkerSchema[] = [
   {
     name: 'Jank',
     display: ['marker-table', 'marker-chart'],
@@ -99,6 +149,10 @@ export const markerSchemaFrontEndOnly: MarkerSchema[] = [
   },
 ];
 
+export const markerSchemaFrontEndOnly = addPIICategoriesToMarkerSchemas(
+  markerSchemaFrontEndOnlyWithoutPII
+);
+
 /**
  * This function takes the intended marker schema for a marker field, and applies
  * the appropriate formatting function.