From 87c97242aec0170863612fb9ab2adf2db1d3d7aa Mon Sep 17 00:00:00 2001 From: TheLarkInn Date: Fri, 28 Aug 2026 03:33:18 +0000 Subject: [PATCH 1/5] Fix reporter telemetry privacy projection Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6318e80-5da9-4858-a147-817e8692f10e --- ...telemetry-privacy_2026-08-28-03-20-00.json | 11 ++ .../src/telemetry/TelemetryAggregate.ts | 2 +- .../src/telemetry/TelemetrySubscriber.ts | 52 +++++--- libraries/reporter/src/test/Telemetry.test.ts | 125 ++++++++++++++++++ 4 files changed, 169 insertions(+), 21 deletions(-) create mode 100644 common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json diff --git a/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json b/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json new file mode 100644 index 0000000000..5b7d870017 --- /dev/null +++ b/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@rushstack/rush-reporter", + "comment": "Prevent local-sensitive and secret reporter events from contributing producer identities or other values to telemetry aggregates.", + "type": "patch" + } + ], + "packageName": "@rushstack/rush-reporter", + "email": "TheLarkInn@users.noreply.github.com" +} diff --git a/libraries/reporter/src/telemetry/TelemetryAggregate.ts b/libraries/reporter/src/telemetry/TelemetryAggregate.ts index d54fa16977..c484e9dc29 100644 --- a/libraries/reporter/src/telemetry/TelemetryAggregate.ts +++ b/libraries/reporter/src/telemetry/TelemetryAggregate.ts @@ -67,7 +67,7 @@ export interface ITelemetryAggregate { readonly protocolVersion?: IReporterProtocolVersion; /** - * The distinct `packageName@packageVersion` producers observed, sorted. + * The distinct `packageName@packageVersion` producers observed on public envelopes, sorted. */ readonly producerVersions: readonly string[]; } diff --git a/libraries/reporter/src/telemetry/TelemetrySubscriber.ts b/libraries/reporter/src/telemetry/TelemetrySubscriber.ts index 48b45f5098..1cbc6f579f 100644 --- a/libraries/reporter/src/telemetry/TelemetrySubscriber.ts +++ b/libraries/reporter/src/telemetry/TelemetrySubscriber.ts @@ -12,9 +12,10 @@ import type { ITelemetryAggregate, TelemetryResult } from './TelemetryAggregate' * * @remarks * The subscriber runs before reporter filtering, so it observes every event. It - * extracts only allowlisted values: from a diagnostic it keeps the code and - * category but never the parameters, remediation, or templates; it ignores - * messages, raw external output, and command arguments entirely. + * projects envelope metadata and lifecycle values only from public events. From + * a local-sensitive diagnostic it may keep the explicitly public code and + * category, but never parameters, remediation, or templates. It ignores secret + * events, messages, raw external output, and command arguments entirely. * * @beta */ @@ -48,8 +49,34 @@ export class TelemetrySubscriber { * Ingests one event, extracting only allowlisted values. */ public ingest(event: IReporterEventEnvelope): void { - this._protocolVersion = event.protocolVersion; - this._producerVersions.add(`${event.source.packageName}@${event.source.packageVersion}`); + const isPublicEnvelope: boolean = event.privacy === 'public'; + if (isPublicEnvelope) { + this._protocolVersion = event.protocolVersion; + this._producerVersions.add(`${event.source.packageName}@${event.source.packageVersion}`); + } + + if (event.type === 'diagnosticEmitted') { + if (event.privacy !== 'secret') { + // Code and category are public schema fields even when classified + // parameters make the diagnostic envelope local-sensitive. + const payload: { code?: string; category?: string } = event.payload as { + code?: string; + category?: string; + }; + if (payload.code !== undefined) { + this._diagnosticCodes.add(payload.code); + } + if (payload.category !== undefined) { + this._diagnosticCategoryCounts[payload.category] = + (this._diagnosticCategoryCounts[payload.category] ?? 0) + 1; + } + } + return; + } + + if (!isPublicEnvelope) { + return; + } switch (event.type) { case 'commandStarted': { @@ -114,21 +141,6 @@ export class TelemetrySubscriber { this._operationStatuses.set(payload.operationId, payload.status); break; } - case 'diagnosticEmitted': { - // Keeps only the code and category, never parameters, remediation, or templates. - const payload: { code?: string; category?: string } = event.payload as { - code?: string; - category?: string; - }; - if (payload.code !== undefined) { - this._diagnosticCodes.add(payload.code); - } - if (payload.category !== undefined) { - this._diagnosticCategoryCounts[payload.category] = - (this._diagnosticCategoryCounts[payload.category] ?? 0) + 1; - } - break; - } default: { // Messages, raw external output, artifacts, and extension events are not // telemetry. diff --git a/libraries/reporter/src/test/Telemetry.test.ts b/libraries/reporter/src/test/Telemetry.test.ts index 423b8a78cf..4ddcfc5b66 100644 --- a/libraries/reporter/src/test/Telemetry.test.ts +++ b/libraries/reporter/src/test/Telemetry.test.ts @@ -110,6 +110,131 @@ describe('TelemetrySubscriber', () => { } }); + it('does not collect producer identities from local-sensitive or secret extension events', async () => { + const LOCAL_PRIVATE_SOURCE: IReporterEventSource = { + packageName: '@private/local-reporter-plugin', + packageVersion: '1.2.3-private' + }; + const SECRET_PRIVATE_SOURCE: IReporterEventSource = { + packageName: '@private/secret-reporter-plugin', + packageVersion: '4.5.6-secret' + }; + const telemetry: TelemetrySubscriber = new TelemetrySubscriber(); + const manager: ReporterManager = new ReporterManager(); + manager.addReporter(createTelemetryReporter(telemetry)); + await manager.initializeAsync(); + + manager.emit({ + ...rawInput('extension', { name: 'private.local.event', privateField: 'local-private-value' }), + source: LOCAL_PRIVATE_SOURCE, + privacy: 'local-sensitive' + }); + manager.emit({ + ...rawInput('extension', { name: 'private.secret.event', secretField: 'secret-private-value' }), + source: SECRET_PRIVATE_SOURCE, + privacy: 'secret' + }); + await manager.flushAsync(); + + const aggregate: ITelemetryAggregate = telemetry.buildAggregate(); + const serialized: string = JSON.stringify(aggregate); + expect(aggregate.producerVersions).toEqual([]); + expect(aggregate.protocolVersion).toBeUndefined(); + for (const forbidden of [ + LOCAL_PRIVATE_SOURCE.packageName, + LOCAL_PRIVATE_SOURCE.packageVersion, + SECRET_PRIVATE_SOURCE.packageName, + SECRET_PRIVATE_SOURCE.packageVersion, + 'local-private-value', + 'secret-private-value' + ]) { + expect(serialized).not.toContain(forbidden); + } + }); + + it('projects only public envelopes while aggregating public producers deterministically', async () => { + const PUBLIC_EXTENSION_SOURCE: IReporterEventSource = { + packageName: '@rushstack/public-reporter-plugin', + packageVersion: '1.2.3' + }; + const PRIVATE_FIRST_PARTY_SOURCE: IReporterEventSource = { + packageName: '@microsoft/internal-build-plugin', + packageVersion: '9.8.7-private' + }; + const telemetry: TelemetrySubscriber = new TelemetrySubscriber(); + const manager: ReporterManager = new ReporterManager(); + manager.addReporter(createTelemetryReporter(telemetry)); + await manager.initializeAsync(); + + manager.emit({ + ...rawInput('commandResult', { + commandName: 'private-command', + succeeded: false, + exitCode: 97 + }), + source: PRIVATE_FIRST_PARTY_SOURCE, + privacy: 'local-sensitive', + protocolVersion: { major: 7, minor: 0 } + }); + manager.emit({ + ...rawInput('extension', { name: 'public.plugin.event' }), + source: PUBLIC_EXTENSION_SOURCE + }); + manager.emit(rawInput('commandResult', { commandName: 'build', succeeded: true, exitCode: 0 })); + manager.emit({ + ...rawInput('extension', { name: 'public.plugin.event' }), + source: PUBLIC_EXTENSION_SOURCE + }); + manager.emit(rawInput('diagnosticEmitted', { code: 'RUSH_OPERATION_FAILED', category: 'operation' })); + manager.emit({ + ...rawInput('operationStatusChanged', { + operationId: 'private-operation', + status: 'failure' + }), + source: PRIVATE_FIRST_PARTY_SOURCE, + privacy: 'local-sensitive' + }); + manager.emit({ + ...rawInput('diagnosticEmitted', { + code: 'PRIVATE_INTERNAL_DIAGNOSTIC', + category: 'private-category' + }), + source: PRIVATE_FIRST_PARTY_SOURCE, + privacy: 'secret' + }); + manager.emit(rawInput('operationStatusChanged', { operationId: 'public-operation', status: 'success' })); + manager.emit({ + ...rawInput('extension', { name: 'private.secret.event' }), + source: PRIVATE_FIRST_PARTY_SOURCE, + privacy: 'secret', + protocolVersion: { major: 99, minor: 0 } + }); + await manager.flushAsync(); + + const aggregate: ITelemetryAggregate = telemetry.buildAggregate(); + expect(aggregate).toMatchObject({ + commandName: 'build', + result: 'succeeded', + exitCode: 0, + operationStatusCounts: { success: 1 }, + diagnosticCodes: ['RUSH_OPERATION_FAILED'], + diagnosticCategoryCounts: { operation: 1 }, + protocolVersion: { major: 1, minor: 0 }, + producerVersions: ['@microsoft/rush-lib@5.177.2', '@rushstack/public-reporter-plugin@1.2.3'] + }); + const serialized: string = JSON.stringify(aggregate); + for (const forbidden of [ + PRIVATE_FIRST_PARTY_SOURCE.packageName, + PRIVATE_FIRST_PARTY_SOURCE.packageVersion, + 'private-command', + 'PRIVATE_INTERNAL_DIAGNOSTIC', + 'private-category', + 'private-operation' + ]) { + expect(serialized).not.toContain(forbidden); + } + }); + it('never leaks messages, paths, arguments, remediation, raw output, or secret values', async () => { const SECRET: string = 'sk-super-secret-value'; const LOG_PATH: string = '/home/user/secret/install.log'; From e7d7c4136b05280de9405b9bbe13eed5e641fb98 Mon Sep 17 00:00:00 2001 From: TheLarkInn Date: Fri, 28 Aug 2026 03:37:28 +0000 Subject: [PATCH 2/5] Preserve public diagnostic telemetry fields Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6318e80-5da9-4858-a147-817e8692f10e --- ...telemetry-privacy_2026-08-28-03-20-00.json | 2 +- .../src/telemetry/TelemetrySubscriber.ts | 33 +++++++++---------- libraries/reporter/src/test/Telemetry.test.ts | 16 +++++---- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json b/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json index 5b7d870017..48ef859460 100644 --- a/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json +++ b/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@rushstack/rush-reporter", - "comment": "Prevent local-sensitive and secret reporter events from contributing producer identities or other values to telemetry aggregates.", + "comment": "Prevent non-public reporter events from contributing producer identities or other non-public values to telemetry aggregates.", "type": "patch" } ], diff --git a/libraries/reporter/src/telemetry/TelemetrySubscriber.ts b/libraries/reporter/src/telemetry/TelemetrySubscriber.ts index 1cbc6f579f..719700f77c 100644 --- a/libraries/reporter/src/telemetry/TelemetrySubscriber.ts +++ b/libraries/reporter/src/telemetry/TelemetrySubscriber.ts @@ -13,9 +13,10 @@ import type { ITelemetryAggregate, TelemetryResult } from './TelemetryAggregate' * @remarks * The subscriber runs before reporter filtering, so it observes every event. It * projects envelope metadata and lifecycle values only from public events. From - * a local-sensitive diagnostic it may keep the explicitly public code and - * category, but never parameters, remediation, or templates. It ignores secret - * events, messages, raw external output, and command arguments entirely. + * a diagnostic it keeps the explicitly public code and category regardless of + * the envelope privacy floor, but never parameters, remediation, or templates. + * It ignores all other values from non-public events, messages, raw external + * output, and command arguments entirely. * * @beta */ @@ -56,20 +57,18 @@ export class TelemetrySubscriber { } if (event.type === 'diagnosticEmitted') { - if (event.privacy !== 'secret') { - // Code and category are public schema fields even when classified - // parameters make the diagnostic envelope local-sensitive. - const payload: { code?: string; category?: string } = event.payload as { - code?: string; - category?: string; - }; - if (payload.code !== undefined) { - this._diagnosticCodes.add(payload.code); - } - if (payload.category !== undefined) { - this._diagnosticCategoryCounts[payload.category] = - (this._diagnosticCategoryCounts[payload.category] ?? 0) + 1; - } + // Code and category are public schema fields even when classified + // parameters make the diagnostic envelope non-public. + const payload: { code?: string; category?: string } = event.payload as { + code?: string; + category?: string; + }; + if (payload.code !== undefined) { + this._diagnosticCodes.add(payload.code); + } + if (payload.category !== undefined) { + this._diagnosticCategoryCounts[payload.category] = + (this._diagnosticCategoryCounts[payload.category] ?? 0) + 1; } return; } diff --git a/libraries/reporter/src/test/Telemetry.test.ts b/libraries/reporter/src/test/Telemetry.test.ts index 4ddcfc5b66..860751149b 100644 --- a/libraries/reporter/src/test/Telemetry.test.ts +++ b/libraries/reporter/src/test/Telemetry.test.ts @@ -152,7 +152,7 @@ describe('TelemetrySubscriber', () => { } }); - it('projects only public envelopes while aggregating public producers deterministically', async () => { + it('projects public envelopes while preserving allowlisted diagnostic fields deterministically', async () => { const PUBLIC_EXTENSION_SOURCE: IReporterEventSource = { packageName: '@rushstack/public-reporter-plugin', packageVersion: '1.2.3' @@ -196,8 +196,11 @@ describe('TelemetrySubscriber', () => { }); manager.emit({ ...rawInput('diagnosticEmitted', { - code: 'PRIVATE_INTERNAL_DIAGNOSTIC', - category: 'private-category' + code: 'RUSH_DEPENDENCY_TOOL_FAILED', + category: 'dependency-tool', + parameters: { + token: { value: 'private-secret-value', privacy: 'secret' } + } }), source: PRIVATE_FIRST_PARTY_SOURCE, privacy: 'secret' @@ -217,8 +220,8 @@ describe('TelemetrySubscriber', () => { result: 'succeeded', exitCode: 0, operationStatusCounts: { success: 1 }, - diagnosticCodes: ['RUSH_OPERATION_FAILED'], - diagnosticCategoryCounts: { operation: 1 }, + diagnosticCodes: ['RUSH_DEPENDENCY_TOOL_FAILED', 'RUSH_OPERATION_FAILED'], + diagnosticCategoryCounts: { operation: 1, 'dependency-tool': 1 }, protocolVersion: { major: 1, minor: 0 }, producerVersions: ['@microsoft/rush-lib@5.177.2', '@rushstack/public-reporter-plugin@1.2.3'] }); @@ -227,8 +230,7 @@ describe('TelemetrySubscriber', () => { PRIVATE_FIRST_PARTY_SOURCE.packageName, PRIVATE_FIRST_PARTY_SOURCE.packageVersion, 'private-command', - 'PRIVATE_INTERNAL_DIAGNOSTIC', - 'private-category', + 'private-secret-value', 'private-operation' ]) { expect(serialized).not.toContain(forbidden); From 8888b3d6333059e1e3ee651ef3a3400c14200e2c Mon Sep 17 00:00:00 2001 From: TheLarkInn Date: Fri, 28 Aug 2026 14:48:45 +0000 Subject: [PATCH 3/5] Harden diagnostic telemetry dimensions Validate non-public diagnostic codes against the registry, bucket unknown categories, and bound retained telemetry dimensions deterministically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6318e80-5da9-4858-a147-817e8692f10e --- ...telemetry-privacy_2026-08-28-03-20-00.json | 2 +- common/reviews/api/rush-reporter.api.md | 2 + .../reporter/src/perf/PerformanceBudgets.ts | 16 +- .../src/telemetry/TelemetrySubscriber.ts | 86 +++++++- .../reporter/src/test/Performance.test.ts | 2 + libraries/reporter/src/test/Telemetry.test.ts | 188 ++++++++++++++++++ 6 files changed, 286 insertions(+), 10 deletions(-) diff --git a/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json b/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json index 48ef859460..db56cab3fa 100644 --- a/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json +++ b/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@rushstack/rush-reporter", - "comment": "Prevent non-public reporter events from contributing producer identities or other non-public values to telemetry aggregates.", + "comment": "Prevent non-public reporter events from contributing unvalidated or unbounded values to telemetry aggregates.", "type": "patch" } ], diff --git a/common/reviews/api/rush-reporter.api.md b/common/reviews/api/rush-reporter.api.md index 0d8ccfa2da..ce8897c9b3 100644 --- a/common/reviews/api/rush-reporter.api.md +++ b/common/reviews/api/rush-reporter.api.md @@ -852,6 +852,8 @@ export interface IReporterPerformanceBudgets { readonly maxAiDetailedDiagnostics: number; readonly maxAiOutputBytes: number; readonly maxInteractiveRefreshHz: number; + readonly maxTelemetryDiagnosticCategories: number; + readonly maxTelemetryDiagnosticCodes: number; readonly maxWallTimeRegressionPercent: number; } diff --git a/libraries/reporter/src/perf/PerformanceBudgets.ts b/libraries/reporter/src/perf/PerformanceBudgets.ts index ab9a4c4647..283d6fe605 100644 --- a/libraries/reporter/src/perf/PerformanceBudgets.ts +++ b/libraries/reporter/src/perf/PerformanceBudgets.ts @@ -45,6 +45,18 @@ export interface IReporterPerformanceBudgets { * before summarizing the remainder. Defaults to `20`. */ readonly maxAiDetailedDiagnostics: number; + + /** + * The maximum number of distinct diagnostic codes retained in a telemetry + * aggregate. Defaults to `20`. + */ + readonly maxTelemetryDiagnosticCodes: number; + + /** + * The maximum number of diagnostic category buckets retained in a telemetry + * aggregate. Defaults to `20`. + */ + readonly maxTelemetryDiagnosticCategories: number; } /** @@ -68,7 +80,9 @@ export const REPORTER_PERFORMANCE_BUDGETS: IReporterPerformanceBudgets = { maxAdditionalPeakMemoryBytes: 32 * BYTES_PER_MIB, maxInteractiveRefreshHz: 10, maxAiOutputBytes: 64 * BYTES_PER_KIB, - maxAiDetailedDiagnostics: 20 + maxAiDetailedDiagnostics: 20, + maxTelemetryDiagnosticCodes: 20, + maxTelemetryDiagnosticCategories: 20 }; /** diff --git a/libraries/reporter/src/telemetry/TelemetrySubscriber.ts b/libraries/reporter/src/telemetry/TelemetrySubscriber.ts index 719700f77c..c5da570b2a 100644 --- a/libraries/reporter/src/telemetry/TelemetrySubscriber.ts +++ b/libraries/reporter/src/telemetry/TelemetrySubscriber.ts @@ -5,8 +5,31 @@ import type { IReporterProtocolVersion } from '../events/ReporterProtocolVersion import type { IReporterEventEnvelope } from '../events/IReporterEventEnvelope'; import type { IReporter } from '../manager/IReporter'; import type { IOperationStatusChangedPayload } from '../lifecycle/LifecycleEvents'; +import { + isValidRushDiagnosticCode, + RUSH_DIAGNOSTIC_CODE_DEFINITIONS, + type IRushDiagnosticCodeDefinition +} from '../diagnostics/RushDiagnosticCodeRegistry'; +import { REPORTER_PERFORMANCE_BUDGETS } from '../perf/PerformanceBudgets'; import type { ITelemetryAggregate, TelemetryResult } from './TelemetryAggregate'; +const OTHER_DIAGNOSTIC_CATEGORY: 'other' = 'other'; +const KNOWN_DIAGNOSTIC_CATEGORIES: ReadonlySet = new Set( + RUSH_DIAGNOSTIC_CODE_DEFINITIONS.map( + (definition: IRushDiagnosticCodeDefinition): string => definition.category + ) +); + +function compareDiagnosticCodeCandidates( + left: readonly [code: string, registered: boolean], + right: readonly [code: string, registered: boolean] +): number { + if (left[1] !== right[1]) { + return left[1] ? -1 : 1; + } + return left[0] < right[0] ? -1 : left[0] > right[0] ? 1 : 0; +} + /** * Consumes canonical events and produces the allowlisted telemetry aggregate. * @@ -29,13 +52,13 @@ export class TelemetrySubscriber { private _protocolVersion: IReporterProtocolVersion | undefined; private readonly _operationStatuses: Map; private readonly _diagnosticCategoryCounts: { [category: string]: number }; - private readonly _diagnosticCodes: Set; + private readonly _diagnosticCodes: Map; private readonly _producerVersions: Set; public constructor() { this._operationStatuses = new Map(); this._diagnosticCategoryCounts = {}; - this._diagnosticCodes = new Set(); + this._diagnosticCodes = new Map(); this._producerVersions = new Set(); } @@ -63,12 +86,17 @@ export class TelemetrySubscriber { code?: string; category?: string; }; - if (payload.code !== undefined) { - this._diagnosticCodes.add(payload.code); + if (typeof payload.code === 'string' && isValidRushDiagnosticCode(payload.code)) { + const registeredDefinition: IRushDiagnosticCodeDefinition | undefined = + RUSH_DIAGNOSTIC_CODE_DEFINITIONS.find( + (definition: IRushDiagnosticCodeDefinition): boolean => definition.code === payload.code + ); + if (isPublicEnvelope || registeredDefinition !== undefined) { + this._recordDiagnosticCode(payload.code, registeredDefinition !== undefined); + } } - if (payload.category !== undefined) { - this._diagnosticCategoryCounts[payload.category] = - (this._diagnosticCategoryCounts[payload.category] ?? 0) + 1; + if (typeof payload.category === 'string') { + this._recordDiagnosticCategory(payload.category); } return; } @@ -170,7 +198,7 @@ export class TelemetrySubscriber { producerVersions: string[]; } = { operationStatusCounts, - diagnosticCodes: [...this._diagnosticCodes].sort(), + diagnosticCodes: [...this._diagnosticCodes.keys()].sort(), diagnosticCategoryCounts: { ...this._diagnosticCategoryCounts }, producerVersions: [...this._producerVersions].sort() }; @@ -196,6 +224,48 @@ export class TelemetrySubscriber { return aggregate; } + + private _recordDiagnosticCode(code: string, registered: boolean): void { + const existingRegistration: boolean | undefined = this._diagnosticCodes.get(code); + if (existingRegistration !== undefined) { + if (registered && !existingRegistration) { + this._diagnosticCodes.set(code, true); + } + return; + } + + if (this._diagnosticCodes.size < REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCodes) { + this._diagnosticCodes.set(code, registered); + return; + } + + let worstCandidate: readonly [code: string, registered: boolean] | undefined; + for (const candidate of this._diagnosticCodes) { + if (worstCandidate === undefined || compareDiagnosticCodeCandidates(candidate, worstCandidate) > 0) { + worstCandidate = candidate; + } + } + + const newCandidate: readonly [code: string, registered: boolean] = [code, registered]; + if (worstCandidate !== undefined && compareDiagnosticCodeCandidates(newCandidate, worstCandidate) < 0) { + this._diagnosticCodes.delete(worstCandidate[0]); + this._diagnosticCodes.set(code, registered); + } + } + + private _recordDiagnosticCategory(category: string): void { + let safeCategory: string = KNOWN_DIAGNOSTIC_CATEGORIES.has(category) + ? category + : OTHER_DIAGNOSTIC_CATEGORY; + if ( + this._diagnosticCategoryCounts[safeCategory] === undefined && + Object.keys(this._diagnosticCategoryCounts).length >= + REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCategories + ) { + safeCategory = OTHER_DIAGNOSTIC_CATEGORY; + } + this._diagnosticCategoryCounts[safeCategory] = (this._diagnosticCategoryCounts[safeCategory] ?? 0) + 1; + } } /** diff --git a/libraries/reporter/src/test/Performance.test.ts b/libraries/reporter/src/test/Performance.test.ts index 47c50adf89..fd05368d32 100644 --- a/libraries/reporter/src/test/Performance.test.ts +++ b/libraries/reporter/src/test/Performance.test.ts @@ -118,6 +118,8 @@ describe('reporter performance budgets', () => { expect(REPORTER_PERFORMANCE_BUDGETS.maxInteractiveRefreshHz).toBe(10); expect(REPORTER_PERFORMANCE_BUDGETS.maxAiOutputBytes).toBe(64 * 1024); expect(REPORTER_PERFORMANCE_BUDGETS.maxAiDetailedDiagnostics).toBe(20); + expect(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCodes).toBe(20); + expect(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCategories).toBe(20); }); it('evaluates wall-time regression against the 3 percent budget', () => { diff --git a/libraries/reporter/src/test/Telemetry.test.ts b/libraries/reporter/src/test/Telemetry.test.ts index 860751149b..4c7d8b6275 100644 --- a/libraries/reporter/src/test/Telemetry.test.ts +++ b/libraries/reporter/src/test/Telemetry.test.ts @@ -5,6 +5,7 @@ import { TelemetrySubscriber, createTelemetryReporter, createBeforeLogAdapter, + REPORTER_PERFORMANCE_BUDGETS, TELEMETRY_AGGREGATE_KEYS, LifecycleEmitter, ReporterManager, @@ -51,6 +52,25 @@ function rawInput(type: string, payload: unknown): IReporterEmitEventInput['privacy'], + payload: unknown +): IReporterEventEnvelope { + return { + protocolVersion: { major: 1, minor: 0 }, + eventId: `foreign_${sequence}`, + sessionId: 'foreign-session', + sequence, + timestamp: '2026-08-28T00:00:00.000Z', + source: { packageName: '@foreign/reporter-plugin', packageVersion: '1.0.0' }, + privacy, + required: true, + type: 'diagnosticEmitted', + payload + }; +} + describe('TelemetrySubscriber', () => { it('produces an allowlisted aggregate from the event stream before reporter filtering', async () => { const telemetry: TelemetrySubscriber = new TelemetrySubscriber(); @@ -152,6 +172,174 @@ describe('TelemetrySubscriber', () => { } }); + it('rejects hostile non-public diagnostic fields from foreign envelopes', async () => { + const TOKEN_CODE: string = 'ghp_super_secret_token'; + const TOKEN_CATEGORY: string = 'token=super-secret-value'; + const PATH_CODE: string = '/home/user/private/.npmrc'; + const PATH_CATEGORY: string = 'C:\\Users\\private\\rush.json'; + const telemetry: TelemetrySubscriber = new TelemetrySubscriber(); + const manager: ReporterManager = new ReporterManager(); + manager.addReporter(createTelemetryReporter(telemetry)); + await manager.initializeAsync(); + + manager.ingestForeignEnvelope( + foreignDiagnosticEnvelope(1, 'local-sensitive', { + code: PATH_CODE, + category: TOKEN_CATEGORY + }) + ); + manager.ingestForeignEnvelope( + foreignDiagnosticEnvelope(2, 'secret', { + code: TOKEN_CODE, + category: PATH_CATEGORY + }) + ); + manager.ingestForeignEnvelope( + foreignDiagnosticEnvelope(3, 'local-sensitive', { + code: 'RUSH_OPERATION_FAILED', + category: 'operation' + }) + ); + manager.ingestForeignEnvelope( + foreignDiagnosticEnvelope(4, 'secret', { + code: 'RUSH_DEPENDENCY_TOOL_FAILED', + category: 'dependency-tool' + }) + ); + await manager.flushAsync(); + + const aggregate: ITelemetryAggregate = telemetry.buildAggregate(); + expect(aggregate.diagnosticCodes).toEqual(['RUSH_DEPENDENCY_TOOL_FAILED', 'RUSH_OPERATION_FAILED']); + expect(aggregate.diagnosticCategoryCounts).toEqual({ + other: 2, + operation: 1, + 'dependency-tool': 1 + }); + const serialized: string = JSON.stringify(aggregate); + for (const forbidden of [TOKEN_CODE, TOKEN_CATEGORY, PATH_CODE, PATH_CATEGORY]) { + expect(serialized).not.toContain(forbidden); + } + }); + + it('preserves allowlisted diagnostics across mixed privacy ordering', async () => { + const telemetry: TelemetrySubscriber = new TelemetrySubscriber(); + const manager: ReporterManager = new ReporterManager(); + manager.addReporter(createTelemetryReporter(telemetry)); + await manager.initializeAsync(); + + manager.ingestForeignEnvelope( + foreignDiagnosticEnvelope(1, 'secret', { + code: 'RUSH_DEPENDENCY_TOOL_FAILED', + category: 'dependency-tool' + }) + ); + manager.ingestForeignEnvelope( + foreignDiagnosticEnvelope(2, 'public', { + code: 'RUSH_OPERATION_FAILED', + category: 'operation' + }) + ); + manager.ingestForeignEnvelope( + foreignDiagnosticEnvelope(3, 'local-sensitive', { + code: 'RUSH_CONFIG_INVALID_JSON', + category: 'configuration' + }) + ); + manager.ingestForeignEnvelope( + foreignDiagnosticEnvelope(4, 'secret', { + code: 'RUSH_NOT_REGISTERED_PRIVATE', + category: 'future-private-category' + }) + ); + manager.ingestForeignEnvelope( + foreignDiagnosticEnvelope(5, 'public', { + code: 'RUSH_FUTURE_PUBLIC_CODE', + category: 'future-public-category' + }) + ); + await manager.flushAsync(); + + expect(telemetry.buildAggregate()).toMatchObject({ + diagnosticCodes: [ + 'RUSH_CONFIG_INVALID_JSON', + 'RUSH_DEPENDENCY_TOOL_FAILED', + 'RUSH_FUTURE_PUBLIC_CODE', + 'RUSH_OPERATION_FAILED' + ], + diagnosticCategoryCounts: { + configuration: 1, + 'dependency-tool': 1, + operation: 1, + other: 2 + } + }); + }); + + it('bounds diagnostic dimensions deterministically under cardinality flooding', async () => { + const publicCodes: string[] = []; + for ( + let index: number = 0; + index < REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCodes * 3; + index++ + ) { + publicCodes.push(`RUSH_FOREIGN_CODE${String(index).padStart(3, '0')}`); + } + const hostilePrivateCodes: string[] = publicCodes.map((code: string): string => `${code}_PRIVATE`); + const payloads: Array<{ + privacy: IReporterEventEnvelope['privacy']; + code: string; + category: string; + }> = [ + ...publicCodes.map((code: string, index: number) => ({ + privacy: 'public' as const, + code, + category: `/private/category/${index}` + })), + ...hostilePrivateCodes.map((code: string, index: number) => ({ + privacy: index % 2 === 0 ? ('local-sensitive' as const) : ('secret' as const), + code, + category: `token-${index}` + })), + { privacy: 'secret', code: 'RUSH_OPERATION_FAILED', category: 'operation' }, + { + privacy: 'local-sensitive', + code: 'RUSH_DEPENDENCY_TOOL_FAILED', + category: 'dependency-tool' + } + ]; + + async function aggregatePayloads(orderedPayloads: typeof payloads): Promise { + const telemetry: TelemetrySubscriber = new TelemetrySubscriber(); + const manager: ReporterManager = new ReporterManager(); + manager.addReporter(createTelemetryReporter(telemetry)); + await manager.initializeAsync(); + orderedPayloads.forEach((payload, index: number) => { + manager.ingestForeignEnvelope( + foreignDiagnosticEnvelope(index + 1, payload.privacy, { + code: payload.code, + category: payload.category + }) + ); + }); + await manager.flushAsync(); + return telemetry.buildAggregate(); + } + + const forward: ITelemetryAggregate = await aggregatePayloads(payloads); + const reverse: ITelemetryAggregate = await aggregatePayloads([...payloads].reverse()); + expect(reverse.diagnosticCodes).toEqual(forward.diagnosticCodes); + expect(forward.diagnosticCodes).toHaveLength(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCodes); + expect(forward.diagnosticCodes).toContain('RUSH_OPERATION_FAILED'); + expect(forward.diagnosticCodes).toContain('RUSH_DEPENDENCY_TOOL_FAILED'); + expect(forward.diagnosticCodes).not.toContain(hostilePrivateCodes[0]); + expect(forward.diagnosticCategoryCounts).toEqual({ + other: publicCodes.length + hostilePrivateCodes.length, + operation: 1, + 'dependency-tool': 1 + }); + expect(Object.keys(forward.diagnosticCategoryCounts)).toHaveLength(3); + }); + it('projects public envelopes while preserving allowlisted diagnostic fields deterministically', async () => { const PUBLIC_EXTENSION_SOURCE: IReporterEventSource = { packageName: '@rushstack/public-reporter-plugin', From a3505aec1ed738c7afe5315110ecabe72074bf75 Mon Sep 17 00:00:00 2001 From: TheLarkInn Date: Fri, 28 Aug 2026 15:17:47 +0000 Subject: [PATCH 4/5] Harden telemetry session attribution Keep protocol metadata root-owned, gate mixed-privacy diagnostics, and bound producer attribution with trusted deterministic retention. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6318e80-5da9-4858-a147-817e8692f10e --- ...telemetry-privacy_2026-08-28-03-20-00.json | 2 +- common/reviews/api/rush-reporter.api.md | 2 + .../reporter/src/perf/PerformanceBudgets.ts | 16 +- .../src/telemetry/TelemetryAggregate.ts | 9 +- .../src/telemetry/TelemetrySubscriber.ts | 195 +++++++++++------ .../reporter/src/test/Performance.test.ts | 2 + libraries/reporter/src/test/Telemetry.test.ts | 207 ++++++++++++++++-- 7 files changed, 354 insertions(+), 79 deletions(-) diff --git a/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json b/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json index db56cab3fa..34e827da7a 100644 --- a/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json +++ b/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@rushstack/rush-reporter", - "comment": "Prevent non-public reporter events from contributing unvalidated or unbounded values to telemetry aggregates.", + "comment": "Prevent non-public reporter events from contributing unvalidated or unbounded values to telemetry aggregates, including producer identity and protocol metadata.", "type": "patch" } ], diff --git a/common/reviews/api/rush-reporter.api.md b/common/reviews/api/rush-reporter.api.md index ce8897c9b3..70d8580951 100644 --- a/common/reviews/api/rush-reporter.api.md +++ b/common/reviews/api/rush-reporter.api.md @@ -854,6 +854,8 @@ export interface IReporterPerformanceBudgets { readonly maxInteractiveRefreshHz: number; readonly maxTelemetryDiagnosticCategories: number; readonly maxTelemetryDiagnosticCodes: number; + readonly maxTelemetryProducerVersionLength: number; + readonly maxTelemetryProducerVersions: number; readonly maxWallTimeRegressionPercent: number; } diff --git a/libraries/reporter/src/perf/PerformanceBudgets.ts b/libraries/reporter/src/perf/PerformanceBudgets.ts index 283d6fe605..ad35b62e78 100644 --- a/libraries/reporter/src/perf/PerformanceBudgets.ts +++ b/libraries/reporter/src/perf/PerformanceBudgets.ts @@ -57,6 +57,18 @@ export interface IReporterPerformanceBudgets { * aggregate. Defaults to `20`. */ readonly maxTelemetryDiagnosticCategories: number; + + /** + * The maximum number of distinct producer versions retained in a telemetry + * aggregate. Defaults to `20`. + */ + readonly maxTelemetryProducerVersions: number; + + /** + * The maximum character length of one `packageName@packageVersion` telemetry + * entry. Longer entries are omitted. Defaults to `256`. + */ + readonly maxTelemetryProducerVersionLength: number; } /** @@ -82,7 +94,9 @@ export const REPORTER_PERFORMANCE_BUDGETS: IReporterPerformanceBudgets = { maxAiOutputBytes: 64 * BYTES_PER_KIB, maxAiDetailedDiagnostics: 20, maxTelemetryDiagnosticCodes: 20, - maxTelemetryDiagnosticCategories: 20 + maxTelemetryDiagnosticCategories: 20, + maxTelemetryProducerVersions: 20, + maxTelemetryProducerVersionLength: 256 }; /** diff --git a/libraries/reporter/src/telemetry/TelemetryAggregate.ts b/libraries/reporter/src/telemetry/TelemetryAggregate.ts index c484e9dc29..999cea908d 100644 --- a/libraries/reporter/src/telemetry/TelemetryAggregate.ts +++ b/libraries/reporter/src/telemetry/TelemetryAggregate.ts @@ -67,7 +67,14 @@ export interface ITelemetryAggregate { readonly protocolVersion?: IReporterProtocolVersion; /** - * The distinct `packageName@packageVersion` producers observed on public envelopes, sorted. + * The distinct `packageName@packageVersion` producers observed on effectively + * public envelopes, sorted. + * + * @remarks + * The list is bounded by the reporter telemetry budgets. Trusted + * `@microsoft/` and `@rushstack/` producers are retained before other + * producers, remaining entries are selected lexicographically, and entries + * over the per-entry length budget are omitted. */ readonly producerVersions: readonly string[]; } diff --git a/libraries/reporter/src/telemetry/TelemetrySubscriber.ts b/libraries/reporter/src/telemetry/TelemetrySubscriber.ts index c5da570b2a..698898313c 100644 --- a/libraries/reporter/src/telemetry/TelemetrySubscriber.ts +++ b/libraries/reporter/src/telemetry/TelemetrySubscriber.ts @@ -14,15 +14,47 @@ import { REPORTER_PERFORMANCE_BUDGETS } from '../perf/PerformanceBudgets'; import type { ITelemetryAggregate, TelemetryResult } from './TelemetryAggregate'; const OTHER_DIAGNOSTIC_CATEGORY: 'other' = 'other'; +const TRUSTED_PRODUCER_PREFIXES: readonly string[] = ['@microsoft/', '@rushstack/']; +const REGISTERED_DIAGNOSTIC_CODE_DEFINITIONS: ReadonlyMap = new Map( + RUSH_DIAGNOSTIC_CODE_DEFINITIONS.map( + (definition: IRushDiagnosticCodeDefinition): readonly [string, IRushDiagnosticCodeDefinition] => [ + definition.code, + definition + ] + ) +); const KNOWN_DIAGNOSTIC_CATEGORIES: ReadonlySet = new Set( RUSH_DIAGNOSTIC_CODE_DEFINITIONS.map( (definition: IRushDiagnosticCodeDefinition): string => definition.category ) ); -function compareDiagnosticCodeCandidates( - left: readonly [code: string, registered: boolean], - right: readonly [code: string, registered: boolean] +function isRecord(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function isDiagnosticPayloadEffectivelyPublic(payload: unknown): boolean { + if (!isRecord(payload)) { + return false; + } + const parameters: unknown = payload.parameters; + if (parameters === undefined) { + return true; + } + if (!isRecord(parameters)) { + return false; + } + for (const parameter of Object.values(parameters)) { + if (!isRecord(parameter) || parameter.privacy !== 'public') { + return false; + } + } + return true; +} + +function comparePrioritizedCandidates( + left: readonly [value: string, preferred: boolean], + right: readonly [value: string, preferred: boolean] ): number { if (left[1] !== right[1]) { return left[1] ? -1 : 1; @@ -30,16 +62,51 @@ function compareDiagnosticCodeCandidates( return left[0] < right[0] ? -1 : left[0] > right[0] ? 1 : 0; } +function recordBoundedPrioritizedValue( + values: Map, + value: string, + preferred: boolean, + maximumCount: number +): void { + const existingPriority: boolean | undefined = values.get(value); + if (existingPriority !== undefined) { + if (preferred && !existingPriority) { + values.set(value, true); + } + return; + } + + if (values.size < maximumCount) { + values.set(value, preferred); + return; + } + + let worstCandidate: readonly [value: string, preferred: boolean] | undefined; + for (const candidate of values) { + if (worstCandidate === undefined || comparePrioritizedCandidates(candidate, worstCandidate) > 0) { + worstCandidate = candidate; + } + } + + const newCandidate: readonly [value: string, preferred: boolean] = [value, preferred]; + if (worstCandidate !== undefined && comparePrioritizedCandidates(newCandidate, worstCandidate) < 0) { + values.delete(worstCandidate[0]); + values.set(value, preferred); + } +} + /** * Consumes canonical events and produces the allowlisted telemetry aggregate. * * @remarks * The subscriber runs before reporter filtering, so it observes every event. It - * projects envelope metadata and lifecycle values only from public events. From - * a diagnostic it keeps the explicitly public code and category regardless of - * the envelope privacy floor, but never parameters, remediation, or templates. - * It ignores all other values from non-public events, messages, raw external - * output, and command arguments entirely. + * projects envelope metadata and lifecycle values only from effectively public + * events. A diagnostic containing any non-public parameter is treated as + * non-public even when its envelope floor is `public`. From a non-public + * diagnostic it keeps only a registered code and that code's registry category, + * never parameters, remediation, or templates. It ignores all other values from + * non-public events, messages, raw external output, and command arguments + * entirely. * * @beta */ @@ -53,13 +120,13 @@ export class TelemetrySubscriber { private readonly _operationStatuses: Map; private readonly _diagnosticCategoryCounts: { [category: string]: number }; private readonly _diagnosticCodes: Map; - private readonly _producerVersions: Set; + private readonly _producerVersions: Map; public constructor() { this._operationStatuses = new Map(); this._diagnosticCategoryCounts = {}; this._diagnosticCodes = new Map(); - this._producerVersions = new Set(); + this._producerVersions = new Map(); } /** @@ -73,35 +140,41 @@ export class TelemetrySubscriber { * Ingests one event, extracting only allowlisted values. */ public ingest(event: IReporterEventEnvelope): void { - const isPublicEnvelope: boolean = event.privacy === 'public'; - if (isPublicEnvelope) { - this._protocolVersion = event.protocolVersion; - this._producerVersions.add(`${event.source.packageName}@${event.source.packageVersion}`); + const isEffectivelyPublicEnvelope: boolean = + event.privacy === 'public' && + (event.type !== 'diagnosticEmitted' || isDiagnosticPayloadEffectivelyPublic(event.payload)); + if (isEffectivelyPublicEnvelope) { + if (event.parentSessionId === undefined) { + this._protocolVersion = event.protocolVersion; + } + this._recordProducerVersion(event.source.packageName, event.source.packageVersion); } if (event.type === 'diagnosticEmitted') { // Code and category are public schema fields even when classified // parameters make the diagnostic envelope non-public. - const payload: { code?: string; category?: string } = event.payload as { - code?: string; - category?: string; - }; - if (typeof payload.code === 'string' && isValidRushDiagnosticCode(payload.code)) { - const registeredDefinition: IRushDiagnosticCodeDefinition | undefined = - RUSH_DIAGNOSTIC_CODE_DEFINITIONS.find( - (definition: IRushDiagnosticCodeDefinition): boolean => definition.code === payload.code - ); - if (isPublicEnvelope || registeredDefinition !== undefined) { + const payload: { code?: unknown; category?: unknown } = isRecord(event.payload) ? event.payload : {}; + const registeredDefinition: IRushDiagnosticCodeDefinition | undefined = + typeof payload.code === 'string' + ? REGISTERED_DIAGNOSTIC_CODE_DEFINITIONS.get(payload.code) + : undefined; + if (isEffectivelyPublicEnvelope) { + if (typeof payload.code === 'string' && isValidRushDiagnosticCode(payload.code)) { this._recordDiagnosticCode(payload.code, registeredDefinition !== undefined); } - } - if (typeof payload.category === 'string') { - this._recordDiagnosticCategory(payload.category); + if (typeof payload.category === 'string') { + this._recordDiagnosticCategory( + KNOWN_DIAGNOSTIC_CATEGORIES.has(payload.category) ? payload.category : OTHER_DIAGNOSTIC_CATEGORY + ); + } + } else if (registeredDefinition !== undefined) { + this._recordDiagnosticCode(registeredDefinition.code, true); + this._recordDiagnosticCategory(registeredDefinition.category); } return; } - if (!isPublicEnvelope) { + if (!isEffectivelyPublicEnvelope) { return; } @@ -184,6 +257,10 @@ export class TelemetrySubscriber { for (const status of this._operationStatuses.values()) { operationStatusCounts[status] = (operationStatusCounts[status] ?? 0) + 1; } + const diagnosticCategoryCounts: { [category: string]: number } = {}; + for (const category of Object.keys(this._diagnosticCategoryCounts).sort()) { + diagnosticCategoryCounts[category] = this._diagnosticCategoryCounts[category]; + } const aggregate: { commandName?: string; @@ -199,8 +276,8 @@ export class TelemetrySubscriber { } = { operationStatusCounts, diagnosticCodes: [...this._diagnosticCodes.keys()].sort(), - diagnosticCategoryCounts: { ...this._diagnosticCategoryCounts }, - producerVersions: [...this._producerVersions].sort() + diagnosticCategoryCounts, + producerVersions: [...this._producerVersions.keys()].sort() }; if (this._commandName !== undefined) { @@ -226,45 +303,39 @@ export class TelemetrySubscriber { } private _recordDiagnosticCode(code: string, registered: boolean): void { - const existingRegistration: boolean | undefined = this._diagnosticCodes.get(code); - if (existingRegistration !== undefined) { - if (registered && !existingRegistration) { - this._diagnosticCodes.set(code, true); - } - return; - } + recordBoundedPrioritizedValue( + this._diagnosticCodes, + code, + registered, + REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCodes + ); + } - if (this._diagnosticCodes.size < REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCodes) { - this._diagnosticCodes.set(code, registered); + private _recordDiagnosticCategory(category: string): void { + const existingCount: number | undefined = this._diagnosticCategoryCounts[category]; + if (existingCount !== undefined) { + this._diagnosticCategoryCounts[category] = existingCount + 1; return; } - - let worstCandidate: readonly [code: string, registered: boolean] | undefined; - for (const candidate of this._diagnosticCodes) { - if (worstCandidate === undefined || compareDiagnosticCodeCandidates(candidate, worstCandidate) > 0) { - worstCandidate = candidate; - } - } - - const newCandidate: readonly [code: string, registered: boolean] = [code, registered]; - if (worstCandidate !== undefined && compareDiagnosticCodeCandidates(newCandidate, worstCandidate) < 0) { - this._diagnosticCodes.delete(worstCandidate[0]); - this._diagnosticCodes.set(code, registered); + if ( + Object.keys(this._diagnosticCategoryCounts).length < + REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCategories + ) { + this._diagnosticCategoryCounts[category] = 1; } } - private _recordDiagnosticCategory(category: string): void { - let safeCategory: string = KNOWN_DIAGNOSTIC_CATEGORIES.has(category) - ? category - : OTHER_DIAGNOSTIC_CATEGORY; - if ( - this._diagnosticCategoryCounts[safeCategory] === undefined && - Object.keys(this._diagnosticCategoryCounts).length >= - REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCategories - ) { - safeCategory = OTHER_DIAGNOSTIC_CATEGORY; + private _recordProducerVersion(packageName: string, packageVersion: string): void { + const producerVersion: string = `${packageName}@${packageVersion}`; + if (producerVersion.length > REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersionLength) { + return; } - this._diagnosticCategoryCounts[safeCategory] = (this._diagnosticCategoryCounts[safeCategory] ?? 0) + 1; + recordBoundedPrioritizedValue( + this._producerVersions, + producerVersion, + TRUSTED_PRODUCER_PREFIXES.some((prefix: string): boolean => packageName.startsWith(prefix)), + REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersions + ); } } diff --git a/libraries/reporter/src/test/Performance.test.ts b/libraries/reporter/src/test/Performance.test.ts index fd05368d32..0898117580 100644 --- a/libraries/reporter/src/test/Performance.test.ts +++ b/libraries/reporter/src/test/Performance.test.ts @@ -120,6 +120,8 @@ describe('reporter performance budgets', () => { expect(REPORTER_PERFORMANCE_BUDGETS.maxAiDetailedDiagnostics).toBe(20); expect(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCodes).toBe(20); expect(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCategories).toBe(20); + expect(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersions).toBe(20); + expect(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersionLength).toBe(256); }); it('evaluates wall-time regression against the 3 percent budget', () => { diff --git a/libraries/reporter/src/test/Telemetry.test.ts b/libraries/reporter/src/test/Telemetry.test.ts index 4c7d8b6275..db9ece3b8e 100644 --- a/libraries/reporter/src/test/Telemetry.test.ts +++ b/libraries/reporter/src/test/Telemetry.test.ts @@ -52,25 +52,46 @@ function rawInput(type: string, payload: unknown): IReporterEmitEventInput['privacy']; + readonly source?: IReporterEventSource; + readonly protocolVersion?: IReporterEventEnvelope['protocolVersion']; + readonly parentSessionId?: string; +} + +function foreignEnvelope( sequence: number, - privacy: IReporterEventEnvelope['privacy'], - payload: unknown + type: IReporterEventEnvelope['type'], + payload: unknown, + options: IForeignEnvelopeOptions = {} ): IReporterEventEnvelope { return { - protocolVersion: { major: 1, minor: 0 }, + protocolVersion: options.protocolVersion ?? { major: 1, minor: 0 }, eventId: `foreign_${sequence}`, sessionId: 'foreign-session', + parentSessionId: options.parentSessionId, sequence, timestamp: '2026-08-28T00:00:00.000Z', - source: { packageName: '@foreign/reporter-plugin', packageVersion: '1.0.0' }, - privacy, + source: options.source ?? { + packageName: '@foreign/reporter-plugin', + packageVersion: '1.0.0' + }, + privacy: options.privacy ?? 'public', required: true, - type: 'diagnosticEmitted', + type, payload }; } +function foreignDiagnosticEnvelope( + sequence: number, + privacy: IReporterEventEnvelope['privacy'], + payload: unknown, + options: Omit = {} +): IReporterEventEnvelope { + return foreignEnvelope(sequence, 'diagnosticEmitted', payload, { ...options, privacy }); +} + describe('TelemetrySubscriber', () => { it('produces an allowlisted aggregate from the event stream before reporter filtering', async () => { const telemetry: TelemetrySubscriber = new TelemetrySubscriber(); @@ -197,13 +218,13 @@ describe('TelemetrySubscriber', () => { manager.ingestForeignEnvelope( foreignDiagnosticEnvelope(3, 'local-sensitive', { code: 'RUSH_OPERATION_FAILED', - category: 'operation' + category: 'network-auth' }) ); manager.ingestForeignEnvelope( foreignDiagnosticEnvelope(4, 'secret', { code: 'RUSH_DEPENDENCY_TOOL_FAILED', - category: 'dependency-tool' + category: 'configuration' }) ); await manager.flushAsync(); @@ -211,10 +232,10 @@ describe('TelemetrySubscriber', () => { const aggregate: ITelemetryAggregate = telemetry.buildAggregate(); expect(aggregate.diagnosticCodes).toEqual(['RUSH_DEPENDENCY_TOOL_FAILED', 'RUSH_OPERATION_FAILED']); expect(aggregate.diagnosticCategoryCounts).toEqual({ - other: 2, - operation: 1, - 'dependency-tool': 1 + 'dependency-tool': 1, + operation: 1 }); + expect(Object.keys(aggregate.diagnosticCategoryCounts)).toEqual(['dependency-tool', 'operation']); const serialized: string = JSON.stringify(aggregate); for (const forbidden of [TOKEN_CODE, TOKEN_CATEGORY, PATH_CODE, PATH_CATEGORY]) { expect(serialized).not.toContain(forbidden); @@ -270,9 +291,15 @@ describe('TelemetrySubscriber', () => { configuration: 1, 'dependency-tool': 1, operation: 1, - other: 2 + other: 1 } }); + expect(Object.keys(telemetry.buildAggregate().diagnosticCategoryCounts)).toEqual([ + 'configuration', + 'dependency-tool', + 'operation', + 'other' + ]); }); it('bounds diagnostic dimensions deterministically under cardinality flooding', async () => { @@ -333,13 +360,165 @@ describe('TelemetrySubscriber', () => { expect(forward.diagnosticCodes).toContain('RUSH_DEPENDENCY_TOOL_FAILED'); expect(forward.diagnosticCodes).not.toContain(hostilePrivateCodes[0]); expect(forward.diagnosticCategoryCounts).toEqual({ - other: publicCodes.length + hostilePrivateCodes.length, + other: publicCodes.length, operation: 1, 'dependency-tool': 1 }); expect(Object.keys(forward.diagnosticCategoryCounts)).toHaveLength(3); }); + it('keeps protocol root-owned while attributing safe child diagnostics', async () => { + const CHILD_PUBLIC_SOURCE: IReporterEventSource = { + packageName: '@rushstack/heft', + packageVersion: '1.2.19' + }; + const CHILD_SECRET_SOURCE: IReporterEventSource = { + packageName: '@private/child-plugin', + packageVersion: '9.9.9-secret' + }; + const telemetry: TelemetrySubscriber = new TelemetrySubscriber(); + const manager: ReporterManager = new ReporterManager(); + manager.addReporter(createTelemetryReporter(telemetry)); + await manager.initializeAsync(); + + manager.emit(rawInput('commandResult', { commandName: 'build', succeeded: true, exitCode: 0 })); + manager.ingestForeignEnvelope( + foreignDiagnosticEnvelope( + 1, + 'public', + { code: 'RUSH_OPERATION_FAILED', category: 'operation' }, + { + source: CHILD_PUBLIC_SOURCE, + protocolVersion: { major: 99, minor: 1 }, + parentSessionId: 'sess' + } + ) + ); + manager.ingestForeignEnvelope( + foreignDiagnosticEnvelope( + 2, + 'secret', + { code: 'RUSH_DEPENDENCY_TOOL_FAILED', category: 'configuration' }, + { + source: CHILD_SECRET_SOURCE, + protocolVersion: { major: 100, minor: 0 }, + parentSessionId: 'sess' + } + ) + ); + await manager.flushAsync(); + + expect(telemetry.buildAggregate()).toMatchObject({ + protocolVersion: { major: 1, minor: 0 }, + producerVersions: ['@microsoft/rush-lib@5.177.2', '@rushstack/heft@1.2.19'], + diagnosticCodes: ['RUSH_DEPENDENCY_TOOL_FAILED', 'RUSH_OPERATION_FAILED'], + diagnosticCategoryCounts: { 'dependency-tool': 1, operation: 1 } + }); + }); + + it('bounds foreign producer versions by priority, order, and entry length', async () => { + const untrustedSources: IReporterEventSource[] = []; + for ( + let index: number = 0; + index < REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersions * 3; + index++ + ) { + untrustedSources.push({ + packageName: `@foreign/plugin-${String(index).padStart(3, '0')}`, + packageVersion: '1.0.0' + }); + } + const trustedSources: IReporterEventSource[] = [ + { packageName: '@microsoft/rush-lib', packageVersion: '5.177.2' }, + { packageName: '@rushstack/heft', packageVersion: '1.2.19' } + ]; + const oversizedSource: IReporterEventSource = { + packageName: `@microsoft/${'x'.repeat(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersionLength)}`, + packageVersion: '1.0.0' + }; + const sources: IReporterEventSource[] = [...untrustedSources, oversizedSource, ...trustedSources]; + + async function aggregateSources( + orderedSources: readonly IReporterEventSource[] + ): Promise { + const telemetry: TelemetrySubscriber = new TelemetrySubscriber(); + const manager: ReporterManager = new ReporterManager(); + manager.addReporter(createTelemetryReporter(telemetry)); + await manager.initializeAsync(); + orderedSources.forEach((source: IReporterEventSource, index: number) => { + manager.ingestForeignEnvelope( + foreignEnvelope( + index + 1, + 'extension', + { name: 'foreign.public.event' }, + { + source, + parentSessionId: 'sess', + protocolVersion: { major: 50 + index, minor: 0 } + } + ) + ); + }); + await manager.flushAsync(); + return telemetry.buildAggregate(); + } + + const forward: ITelemetryAggregate = await aggregateSources(sources); + const reverse: ITelemetryAggregate = await aggregateSources([...sources].reverse()); + expect(reverse.producerVersions).toEqual(forward.producerVersions); + expect(forward.producerVersions).toHaveLength(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersions); + expect(forward.producerVersions).toContain('@microsoft/rush-lib@5.177.2'); + expect(forward.producerVersions).toContain('@rushstack/heft@1.2.19'); + expect(forward.producerVersions).not.toContain( + `${oversizedSource.packageName}@${oversizedSource.packageVersion}` + ); + for (const producerVersion of forward.producerVersions) { + expect(producerVersion.length).toBeLessThanOrEqual( + REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersionLength + ); + } + expect(forward.protocolVersion).toBeUndefined(); + }); + + it('does not admit producer metadata for lifecycle diagnostics with mixed privacy', async () => { + const SECRET: string = 'mixed-privacy-secret'; + const MIXED_SOURCE: IReporterEventSource = { + packageName: '@private/mixed-diagnostic-plugin', + packageVersion: '1.0.0-private' + }; + const telemetry: TelemetrySubscriber = new TelemetrySubscriber(); + const recording: RecordingReporter = new RecordingReporter(); + const manager: ReporterManager = new ReporterManager(); + manager.addReporter(createTelemetryReporter(telemetry)); + manager.addReporter(recording); + await manager.initializeAsync(); + const emitter: LifecycleEmitter = new LifecycleEmitter({ + sink: manager, + sessionId: 'sess', + source: MIXED_SOURCE, + protocolVersion: { major: 7, minor: 0 } + }); + + emitter.emitDiagnostic( + createRushDiagnostic('RUSH_OPERATION_FAILED', { + parameters: { + publicValue: { value: 'safe', privacy: 'public' }, + token: { value: SECRET, privacy: 'secret' } + } + }) + ); + await manager.flushAsync(); + + expect(recording.reported[0].privacy).toBe('public'); + const aggregate: ITelemetryAggregate = telemetry.buildAggregate(); + expect(aggregate.protocolVersion).toBeUndefined(); + expect(aggregate.producerVersions).toEqual([]); + expect(aggregate.diagnosticCodes).toEqual(['RUSH_OPERATION_FAILED']); + expect(aggregate.diagnosticCategoryCounts).toEqual({ operation: 1 }); + expect(JSON.stringify(aggregate)).not.toContain(SECRET); + expect(JSON.stringify(aggregate)).not.toContain(MIXED_SOURCE.packageName); + }); + it('projects public envelopes while preserving allowlisted diagnostic fields deterministically', async () => { const PUBLIC_EXTENSION_SOURCE: IReporterEventSource = { packageName: '@rushstack/public-reporter-plugin', From 70a28dd51353fc7579c7483c97e327c82eff6f98 Mon Sep 17 00:00:00 2001 From: TheLarkInn Date: Fri, 28 Aug 2026 15:52:21 +0000 Subject: [PATCH 5/5] Protect parent telemetry producers Derive bounded producer retention priority from parent-session provenance instead of child-controlled package namespaces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6318e80-5da9-4858-a147-817e8692f10e --- ...telemetry-privacy_2026-08-28-03-20-00.json | 2 +- .../src/telemetry/TelemetryAggregate.ts | 8 +- .../src/telemetry/TelemetrySubscriber.ts | 15 ++- libraries/reporter/src/test/Telemetry.test.ts | 113 ++++++++++++------ 4 files changed, 95 insertions(+), 43 deletions(-) diff --git a/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json b/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json index 34e827da7a..d854555746 100644 --- a/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json +++ b/common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json @@ -2,7 +2,7 @@ "changes": [ { "packageName": "@rushstack/rush-reporter", - "comment": "Prevent non-public reporter events from contributing unvalidated or unbounded values to telemetry aggregates, including producer identity and protocol metadata.", + "comment": "Prevent non-public reporter events from contributing unvalidated or unbounded values to telemetry aggregates, and protect parent-owned producer and protocol metadata.", "type": "patch" } ], diff --git a/libraries/reporter/src/telemetry/TelemetryAggregate.ts b/libraries/reporter/src/telemetry/TelemetryAggregate.ts index 999cea908d..12ab291610 100644 --- a/libraries/reporter/src/telemetry/TelemetryAggregate.ts +++ b/libraries/reporter/src/telemetry/TelemetryAggregate.ts @@ -71,10 +71,10 @@ export interface ITelemetryAggregate { * public envelopes, sorted. * * @remarks - * The list is bounded by the reporter telemetry budgets. Trusted - * `@microsoft/` and `@rushstack/` producers are retained before other - * producers, remaining entries are selected lexicographically, and entries - * over the per-entry length budget are omitted. + * The list is bounded by the reporter telemetry budgets. Parent-session + * producers are retained before child-session producers, remaining entries + * are selected lexicographically, and entries over the per-entry length + * budget are omitted. Package namespace text does not confer priority. */ readonly producerVersions: readonly string[]; } diff --git a/libraries/reporter/src/telemetry/TelemetrySubscriber.ts b/libraries/reporter/src/telemetry/TelemetrySubscriber.ts index 698898313c..7a6d2230ef 100644 --- a/libraries/reporter/src/telemetry/TelemetrySubscriber.ts +++ b/libraries/reporter/src/telemetry/TelemetrySubscriber.ts @@ -14,7 +14,6 @@ import { REPORTER_PERFORMANCE_BUDGETS } from '../perf/PerformanceBudgets'; import type { ITelemetryAggregate, TelemetryResult } from './TelemetryAggregate'; const OTHER_DIAGNOSTIC_CATEGORY: 'other' = 'other'; -const TRUSTED_PRODUCER_PREFIXES: readonly string[] = ['@microsoft/', '@rushstack/']; const REGISTERED_DIAGNOSTIC_CODE_DEFINITIONS: ReadonlyMap = new Map( RUSH_DIAGNOSTIC_CODE_DEFINITIONS.map( (definition: IRushDiagnosticCodeDefinition): readonly [string, IRushDiagnosticCodeDefinition] => [ @@ -147,7 +146,11 @@ export class TelemetrySubscriber { if (event.parentSessionId === undefined) { this._protocolVersion = event.protocolVersion; } - this._recordProducerVersion(event.source.packageName, event.source.packageVersion); + this._recordProducerVersion( + event.source.packageName, + event.source.packageVersion, + event.parentSessionId === undefined + ); } if (event.type === 'diagnosticEmitted') { @@ -325,7 +328,11 @@ export class TelemetrySubscriber { } } - private _recordProducerVersion(packageName: string, packageVersion: string): void { + private _recordProducerVersion( + packageName: string, + packageVersion: string, + isParentSessionProducer: boolean + ): void { const producerVersion: string = `${packageName}@${packageVersion}`; if (producerVersion.length > REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersionLength) { return; @@ -333,7 +340,7 @@ export class TelemetrySubscriber { recordBoundedPrioritizedValue( this._producerVersions, producerVersion, - TRUSTED_PRODUCER_PREFIXES.some((prefix: string): boolean => packageName.startsWith(prefix)), + isParentSessionProducer, REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersions ); } diff --git a/libraries/reporter/src/test/Telemetry.test.ts b/libraries/reporter/src/test/Telemetry.test.ts index db9ece3b8e..b271623c4a 100644 --- a/libraries/reporter/src/test/Telemetry.test.ts +++ b/libraries/reporter/src/test/Telemetry.test.ts @@ -416,68 +416,113 @@ describe('TelemetrySubscriber', () => { }); }); - it('bounds foreign producer versions by priority, order, and entry length', async () => { - const untrustedSources: IReporterEventSource[] = []; + it('protects parent producers from root-first and root-last spoofed-prefix floods', async () => { + const childSources: IReporterEventSource[] = []; for ( let index: number = 0; index < REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersions * 3; index++ ) { - untrustedSources.push({ - packageName: `@foreign/plugin-${String(index).padStart(3, '0')}`, + childSources.push({ + packageName: + index % 2 === 0 + ? `@microsoft/spoof-${String(index).padStart(3, '0')}` + : `@rushstack/spoof-${String(index).padStart(3, '0')}`, packageVersion: '1.0.0' }); } - const trustedSources: IReporterEventSource[] = [ - { packageName: '@microsoft/rush-lib', packageVersion: '5.177.2' }, - { packageName: '@rushstack/heft', packageVersion: '1.2.19' } + const parentSources: IReporterEventSource[] = [ + { packageName: 'zz-parent-owned-a', packageVersion: '1.0.0' }, + { packageName: 'zz-parent-owned-b', packageVersion: '2.0.0' } ]; - const oversizedSource: IReporterEventSource = { + const oversizedChildSource: IReporterEventSource = { packageName: `@microsoft/${'x'.repeat(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersionLength)}`, packageVersion: '1.0.0' }; - const sources: IReporterEventSource[] = [...untrustedSources, oversizedSource, ...trustedSources]; + const oversizedParentSource: IReporterEventSource = { + packageName: 'z'.repeat(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersionLength), + packageVersion: '1.0.0' + }; async function aggregateSources( - orderedSources: readonly IReporterEventSource[] + rootFirst: boolean, + reverseChildren: boolean ): Promise { const telemetry: TelemetrySubscriber = new TelemetrySubscriber(); const manager: ReporterManager = new ReporterManager(); manager.addReporter(createTelemetryReporter(telemetry)); await manager.initializeAsync(); - orderedSources.forEach((source: IReporterEventSource, index: number) => { - manager.ingestForeignEnvelope( - foreignEnvelope( - index + 1, - 'extension', - { name: 'foreign.public.event' }, - { - source, - parentSessionId: 'sess', - protocolVersion: { major: 50 + index, minor: 0 } - } - ) - ); - }); + + const emitParentSources = (): void => { + for (const source of [...parentSources, oversizedParentSource]) { + manager.emit({ + ...rawInput('extension', { name: 'parent.public.event' }), + source + }); + } + }; + const emitChildSources = (): void => { + const orderedChildren: IReporterEventSource[] = [ + ...(reverseChildren ? [...childSources].reverse() : childSources), + oversizedChildSource + ]; + orderedChildren.forEach((source: IReporterEventSource, index: number) => { + manager.ingestForeignEnvelope( + foreignEnvelope( + index + 1, + 'extension', + { name: 'foreign.public.event' }, + { + source, + parentSessionId: 'sess', + protocolVersion: { major: 50 + index, minor: 0 } + } + ) + ); + }); + }; + + if (rootFirst) { + emitParentSources(); + emitChildSources(); + } else { + emitChildSources(); + emitParentSources(); + } await manager.flushAsync(); return telemetry.buildAggregate(); } - const forward: ITelemetryAggregate = await aggregateSources(sources); - const reverse: ITelemetryAggregate = await aggregateSources([...sources].reverse()); - expect(reverse.producerVersions).toEqual(forward.producerVersions); - expect(forward.producerVersions).toHaveLength(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersions); - expect(forward.producerVersions).toContain('@microsoft/rush-lib@5.177.2'); - expect(forward.producerVersions).toContain('@rushstack/heft@1.2.19'); - expect(forward.producerVersions).not.toContain( - `${oversizedSource.packageName}@${oversizedSource.packageVersion}` + const rootFirstForward: ITelemetryAggregate = await aggregateSources(true, false); + const rootFirstReverse: ITelemetryAggregate = await aggregateSources(true, true); + const rootLastForward: ITelemetryAggregate = await aggregateSources(false, false); + const rootLastReverse: ITelemetryAggregate = await aggregateSources(false, true); + expect(rootFirstReverse.producerVersions).toEqual(rootFirstForward.producerVersions); + expect(rootLastForward.producerVersions).toEqual(rootFirstForward.producerVersions); + expect(rootLastReverse.producerVersions).toEqual(rootFirstForward.producerVersions); + expect(rootFirstForward.producerVersions).toHaveLength( + REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersions + ); + for (const source of parentSources) { + expect(rootFirstForward.producerVersions).toContain(`${source.packageName}@${source.packageVersion}`); + } + expect(rootFirstForward.producerVersions).not.toContain( + `${oversizedChildSource.packageName}@${oversizedChildSource.packageVersion}` + ); + expect(rootFirstForward.producerVersions).not.toContain( + `${oversizedParentSource.packageName}@${oversizedParentSource.packageVersion}` ); - for (const producerVersion of forward.producerVersions) { + expect( + rootFirstForward.producerVersions.filter((producerVersion: string): boolean => + producerVersion.startsWith('@microsoft/spoof-') + ) + ).toHaveLength(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersions - parentSources.length); + for (const producerVersion of rootFirstForward.producerVersions) { expect(producerVersion.length).toBeLessThanOrEqual( REPORTER_PERFORMANCE_BUDGETS.maxTelemetryProducerVersionLength ); } - expect(forward.protocolVersion).toBeUndefined(); + expect(rootFirstForward.protocolVersion).toEqual({ major: 1, minor: 0 }); }); it('does not admit producer metadata for lifecycle diagnostics with mixed privacy', async () => {