Skip to content

Commit 8888b3d

Browse files
committed
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
1 parent e7d7c41 commit 8888b3d

6 files changed

Lines changed: 286 additions & 10 deletions

File tree

common/changes/@rushstack/rush-reporter/copilot-reporter-telemetry-privacy_2026-08-28-03-20-00.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
"changes": [
33
{
44
"packageName": "@rushstack/rush-reporter",
5-
"comment": "Prevent non-public reporter events from contributing producer identities or other non-public values to telemetry aggregates.",
5+
"comment": "Prevent non-public reporter events from contributing unvalidated or unbounded values to telemetry aggregates.",
66
"type": "patch"
77
}
88
],

common/reviews/api/rush-reporter.api.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,8 @@ export interface IReporterPerformanceBudgets {
852852
readonly maxAiDetailedDiagnostics: number;
853853
readonly maxAiOutputBytes: number;
854854
readonly maxInteractiveRefreshHz: number;
855+
readonly maxTelemetryDiagnosticCategories: number;
856+
readonly maxTelemetryDiagnosticCodes: number;
855857
readonly maxWallTimeRegressionPercent: number;
856858
}
857859

libraries/reporter/src/perf/PerformanceBudgets.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,18 @@ export interface IReporterPerformanceBudgets {
4545
* before summarizing the remainder. Defaults to `20`.
4646
*/
4747
readonly maxAiDetailedDiagnostics: number;
48+
49+
/**
50+
* The maximum number of distinct diagnostic codes retained in a telemetry
51+
* aggregate. Defaults to `20`.
52+
*/
53+
readonly maxTelemetryDiagnosticCodes: number;
54+
55+
/**
56+
* The maximum number of diagnostic category buckets retained in a telemetry
57+
* aggregate. Defaults to `20`.
58+
*/
59+
readonly maxTelemetryDiagnosticCategories: number;
4860
}
4961

5062
/**
@@ -68,7 +80,9 @@ export const REPORTER_PERFORMANCE_BUDGETS: IReporterPerformanceBudgets = {
6880
maxAdditionalPeakMemoryBytes: 32 * BYTES_PER_MIB,
6981
maxInteractiveRefreshHz: 10,
7082
maxAiOutputBytes: 64 * BYTES_PER_KIB,
71-
maxAiDetailedDiagnostics: 20
83+
maxAiDetailedDiagnostics: 20,
84+
maxTelemetryDiagnosticCodes: 20,
85+
maxTelemetryDiagnosticCategories: 20
7286
};
7387

7488
/**

libraries/reporter/src/telemetry/TelemetrySubscriber.ts

Lines changed: 78 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,31 @@ import type { IReporterProtocolVersion } from '../events/ReporterProtocolVersion
55
import type { IReporterEventEnvelope } from '../events/IReporterEventEnvelope';
66
import type { IReporter } from '../manager/IReporter';
77
import type { IOperationStatusChangedPayload } from '../lifecycle/LifecycleEvents';
8+
import {
9+
isValidRushDiagnosticCode,
10+
RUSH_DIAGNOSTIC_CODE_DEFINITIONS,
11+
type IRushDiagnosticCodeDefinition
12+
} from '../diagnostics/RushDiagnosticCodeRegistry';
13+
import { REPORTER_PERFORMANCE_BUDGETS } from '../perf/PerformanceBudgets';
814
import type { ITelemetryAggregate, TelemetryResult } from './TelemetryAggregate';
915

16+
const OTHER_DIAGNOSTIC_CATEGORY: 'other' = 'other';
17+
const KNOWN_DIAGNOSTIC_CATEGORIES: ReadonlySet<string> = new Set(
18+
RUSH_DIAGNOSTIC_CODE_DEFINITIONS.map(
19+
(definition: IRushDiagnosticCodeDefinition): string => definition.category
20+
)
21+
);
22+
23+
function compareDiagnosticCodeCandidates(
24+
left: readonly [code: string, registered: boolean],
25+
right: readonly [code: string, registered: boolean]
26+
): number {
27+
if (left[1] !== right[1]) {
28+
return left[1] ? -1 : 1;
29+
}
30+
return left[0] < right[0] ? -1 : left[0] > right[0] ? 1 : 0;
31+
}
32+
1033
/**
1134
* Consumes canonical events and produces the allowlisted telemetry aggregate.
1235
*
@@ -29,13 +52,13 @@ export class TelemetrySubscriber {
2952
private _protocolVersion: IReporterProtocolVersion | undefined;
3053
private readonly _operationStatuses: Map<string, IOperationStatusChangedPayload['status']>;
3154
private readonly _diagnosticCategoryCounts: { [category: string]: number };
32-
private readonly _diagnosticCodes: Set<string>;
55+
private readonly _diagnosticCodes: Map<string, boolean>;
3356
private readonly _producerVersions: Set<string>;
3457

3558
public constructor() {
3659
this._operationStatuses = new Map();
3760
this._diagnosticCategoryCounts = {};
38-
this._diagnosticCodes = new Set();
61+
this._diagnosticCodes = new Map();
3962
this._producerVersions = new Set();
4063
}
4164

@@ -63,12 +86,17 @@ export class TelemetrySubscriber {
6386
code?: string;
6487
category?: string;
6588
};
66-
if (payload.code !== undefined) {
67-
this._diagnosticCodes.add(payload.code);
89+
if (typeof payload.code === 'string' && isValidRushDiagnosticCode(payload.code)) {
90+
const registeredDefinition: IRushDiagnosticCodeDefinition | undefined =
91+
RUSH_DIAGNOSTIC_CODE_DEFINITIONS.find(
92+
(definition: IRushDiagnosticCodeDefinition): boolean => definition.code === payload.code
93+
);
94+
if (isPublicEnvelope || registeredDefinition !== undefined) {
95+
this._recordDiagnosticCode(payload.code, registeredDefinition !== undefined);
96+
}
6897
}
69-
if (payload.category !== undefined) {
70-
this._diagnosticCategoryCounts[payload.category] =
71-
(this._diagnosticCategoryCounts[payload.category] ?? 0) + 1;
98+
if (typeof payload.category === 'string') {
99+
this._recordDiagnosticCategory(payload.category);
72100
}
73101
return;
74102
}
@@ -170,7 +198,7 @@ export class TelemetrySubscriber {
170198
producerVersions: string[];
171199
} = {
172200
operationStatusCounts,
173-
diagnosticCodes: [...this._diagnosticCodes].sort(),
201+
diagnosticCodes: [...this._diagnosticCodes.keys()].sort(),
174202
diagnosticCategoryCounts: { ...this._diagnosticCategoryCounts },
175203
producerVersions: [...this._producerVersions].sort()
176204
};
@@ -196,6 +224,48 @@ export class TelemetrySubscriber {
196224

197225
return aggregate;
198226
}
227+
228+
private _recordDiagnosticCode(code: string, registered: boolean): void {
229+
const existingRegistration: boolean | undefined = this._diagnosticCodes.get(code);
230+
if (existingRegistration !== undefined) {
231+
if (registered && !existingRegistration) {
232+
this._diagnosticCodes.set(code, true);
233+
}
234+
return;
235+
}
236+
237+
if (this._diagnosticCodes.size < REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCodes) {
238+
this._diagnosticCodes.set(code, registered);
239+
return;
240+
}
241+
242+
let worstCandidate: readonly [code: string, registered: boolean] | undefined;
243+
for (const candidate of this._diagnosticCodes) {
244+
if (worstCandidate === undefined || compareDiagnosticCodeCandidates(candidate, worstCandidate) > 0) {
245+
worstCandidate = candidate;
246+
}
247+
}
248+
249+
const newCandidate: readonly [code: string, registered: boolean] = [code, registered];
250+
if (worstCandidate !== undefined && compareDiagnosticCodeCandidates(newCandidate, worstCandidate) < 0) {
251+
this._diagnosticCodes.delete(worstCandidate[0]);
252+
this._diagnosticCodes.set(code, registered);
253+
}
254+
}
255+
256+
private _recordDiagnosticCategory(category: string): void {
257+
let safeCategory: string = KNOWN_DIAGNOSTIC_CATEGORIES.has(category)
258+
? category
259+
: OTHER_DIAGNOSTIC_CATEGORY;
260+
if (
261+
this._diagnosticCategoryCounts[safeCategory] === undefined &&
262+
Object.keys(this._diagnosticCategoryCounts).length >=
263+
REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCategories
264+
) {
265+
safeCategory = OTHER_DIAGNOSTIC_CATEGORY;
266+
}
267+
this._diagnosticCategoryCounts[safeCategory] = (this._diagnosticCategoryCounts[safeCategory] ?? 0) + 1;
268+
}
199269
}
200270

201271
/**

libraries/reporter/src/test/Performance.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ describe('reporter performance budgets', () => {
118118
expect(REPORTER_PERFORMANCE_BUDGETS.maxInteractiveRefreshHz).toBe(10);
119119
expect(REPORTER_PERFORMANCE_BUDGETS.maxAiOutputBytes).toBe(64 * 1024);
120120
expect(REPORTER_PERFORMANCE_BUDGETS.maxAiDetailedDiagnostics).toBe(20);
121+
expect(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCodes).toBe(20);
122+
expect(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCategories).toBe(20);
121123
});
122124

123125
it('evaluates wall-time regression against the 3 percent budget', () => {

libraries/reporter/src/test/Telemetry.test.ts

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
TelemetrySubscriber,
66
createTelemetryReporter,
77
createBeforeLogAdapter,
8+
REPORTER_PERFORMANCE_BUDGETS,
89
TELEMETRY_AGGREGATE_KEYS,
910
LifecycleEmitter,
1011
ReporterManager,
@@ -51,6 +52,25 @@ function rawInput(type: string, payload: unknown): IReporterEmitEventInput<unkno
5152
};
5253
}
5354

55+
function foreignDiagnosticEnvelope(
56+
sequence: number,
57+
privacy: IReporterEventEnvelope<unknown>['privacy'],
58+
payload: unknown
59+
): IReporterEventEnvelope<unknown> {
60+
return {
61+
protocolVersion: { major: 1, minor: 0 },
62+
eventId: `foreign_${sequence}`,
63+
sessionId: 'foreign-session',
64+
sequence,
65+
timestamp: '2026-08-28T00:00:00.000Z',
66+
source: { packageName: '@foreign/reporter-plugin', packageVersion: '1.0.0' },
67+
privacy,
68+
required: true,
69+
type: 'diagnosticEmitted',
70+
payload
71+
};
72+
}
73+
5474
describe('TelemetrySubscriber', () => {
5575
it('produces an allowlisted aggregate from the event stream before reporter filtering', async () => {
5676
const telemetry: TelemetrySubscriber = new TelemetrySubscriber();
@@ -152,6 +172,174 @@ describe('TelemetrySubscriber', () => {
152172
}
153173
});
154174

175+
it('rejects hostile non-public diagnostic fields from foreign envelopes', async () => {
176+
const TOKEN_CODE: string = 'ghp_super_secret_token';
177+
const TOKEN_CATEGORY: string = 'token=super-secret-value';
178+
const PATH_CODE: string = '/home/user/private/.npmrc';
179+
const PATH_CATEGORY: string = 'C:\\Users\\private\\rush.json';
180+
const telemetry: TelemetrySubscriber = new TelemetrySubscriber();
181+
const manager: ReporterManager = new ReporterManager();
182+
manager.addReporter(createTelemetryReporter(telemetry));
183+
await manager.initializeAsync();
184+
185+
manager.ingestForeignEnvelope(
186+
foreignDiagnosticEnvelope(1, 'local-sensitive', {
187+
code: PATH_CODE,
188+
category: TOKEN_CATEGORY
189+
})
190+
);
191+
manager.ingestForeignEnvelope(
192+
foreignDiagnosticEnvelope(2, 'secret', {
193+
code: TOKEN_CODE,
194+
category: PATH_CATEGORY
195+
})
196+
);
197+
manager.ingestForeignEnvelope(
198+
foreignDiagnosticEnvelope(3, 'local-sensitive', {
199+
code: 'RUSH_OPERATION_FAILED',
200+
category: 'operation'
201+
})
202+
);
203+
manager.ingestForeignEnvelope(
204+
foreignDiagnosticEnvelope(4, 'secret', {
205+
code: 'RUSH_DEPENDENCY_TOOL_FAILED',
206+
category: 'dependency-tool'
207+
})
208+
);
209+
await manager.flushAsync();
210+
211+
const aggregate: ITelemetryAggregate = telemetry.buildAggregate();
212+
expect(aggregate.diagnosticCodes).toEqual(['RUSH_DEPENDENCY_TOOL_FAILED', 'RUSH_OPERATION_FAILED']);
213+
expect(aggregate.diagnosticCategoryCounts).toEqual({
214+
other: 2,
215+
operation: 1,
216+
'dependency-tool': 1
217+
});
218+
const serialized: string = JSON.stringify(aggregate);
219+
for (const forbidden of [TOKEN_CODE, TOKEN_CATEGORY, PATH_CODE, PATH_CATEGORY]) {
220+
expect(serialized).not.toContain(forbidden);
221+
}
222+
});
223+
224+
it('preserves allowlisted diagnostics across mixed privacy ordering', async () => {
225+
const telemetry: TelemetrySubscriber = new TelemetrySubscriber();
226+
const manager: ReporterManager = new ReporterManager();
227+
manager.addReporter(createTelemetryReporter(telemetry));
228+
await manager.initializeAsync();
229+
230+
manager.ingestForeignEnvelope(
231+
foreignDiagnosticEnvelope(1, 'secret', {
232+
code: 'RUSH_DEPENDENCY_TOOL_FAILED',
233+
category: 'dependency-tool'
234+
})
235+
);
236+
manager.ingestForeignEnvelope(
237+
foreignDiagnosticEnvelope(2, 'public', {
238+
code: 'RUSH_OPERATION_FAILED',
239+
category: 'operation'
240+
})
241+
);
242+
manager.ingestForeignEnvelope(
243+
foreignDiagnosticEnvelope(3, 'local-sensitive', {
244+
code: 'RUSH_CONFIG_INVALID_JSON',
245+
category: 'configuration'
246+
})
247+
);
248+
manager.ingestForeignEnvelope(
249+
foreignDiagnosticEnvelope(4, 'secret', {
250+
code: 'RUSH_NOT_REGISTERED_PRIVATE',
251+
category: 'future-private-category'
252+
})
253+
);
254+
manager.ingestForeignEnvelope(
255+
foreignDiagnosticEnvelope(5, 'public', {
256+
code: 'RUSH_FUTURE_PUBLIC_CODE',
257+
category: 'future-public-category'
258+
})
259+
);
260+
await manager.flushAsync();
261+
262+
expect(telemetry.buildAggregate()).toMatchObject({
263+
diagnosticCodes: [
264+
'RUSH_CONFIG_INVALID_JSON',
265+
'RUSH_DEPENDENCY_TOOL_FAILED',
266+
'RUSH_FUTURE_PUBLIC_CODE',
267+
'RUSH_OPERATION_FAILED'
268+
],
269+
diagnosticCategoryCounts: {
270+
configuration: 1,
271+
'dependency-tool': 1,
272+
operation: 1,
273+
other: 2
274+
}
275+
});
276+
});
277+
278+
it('bounds diagnostic dimensions deterministically under cardinality flooding', async () => {
279+
const publicCodes: string[] = [];
280+
for (
281+
let index: number = 0;
282+
index < REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCodes * 3;
283+
index++
284+
) {
285+
publicCodes.push(`RUSH_FOREIGN_CODE${String(index).padStart(3, '0')}`);
286+
}
287+
const hostilePrivateCodes: string[] = publicCodes.map((code: string): string => `${code}_PRIVATE`);
288+
const payloads: Array<{
289+
privacy: IReporterEventEnvelope<unknown>['privacy'];
290+
code: string;
291+
category: string;
292+
}> = [
293+
...publicCodes.map((code: string, index: number) => ({
294+
privacy: 'public' as const,
295+
code,
296+
category: `/private/category/${index}`
297+
})),
298+
...hostilePrivateCodes.map((code: string, index: number) => ({
299+
privacy: index % 2 === 0 ? ('local-sensitive' as const) : ('secret' as const),
300+
code,
301+
category: `token-${index}`
302+
})),
303+
{ privacy: 'secret', code: 'RUSH_OPERATION_FAILED', category: 'operation' },
304+
{
305+
privacy: 'local-sensitive',
306+
code: 'RUSH_DEPENDENCY_TOOL_FAILED',
307+
category: 'dependency-tool'
308+
}
309+
];
310+
311+
async function aggregatePayloads(orderedPayloads: typeof payloads): Promise<ITelemetryAggregate> {
312+
const telemetry: TelemetrySubscriber = new TelemetrySubscriber();
313+
const manager: ReporterManager = new ReporterManager();
314+
manager.addReporter(createTelemetryReporter(telemetry));
315+
await manager.initializeAsync();
316+
orderedPayloads.forEach((payload, index: number) => {
317+
manager.ingestForeignEnvelope(
318+
foreignDiagnosticEnvelope(index + 1, payload.privacy, {
319+
code: payload.code,
320+
category: payload.category
321+
})
322+
);
323+
});
324+
await manager.flushAsync();
325+
return telemetry.buildAggregate();
326+
}
327+
328+
const forward: ITelemetryAggregate = await aggregatePayloads(payloads);
329+
const reverse: ITelemetryAggregate = await aggregatePayloads([...payloads].reverse());
330+
expect(reverse.diagnosticCodes).toEqual(forward.diagnosticCodes);
331+
expect(forward.diagnosticCodes).toHaveLength(REPORTER_PERFORMANCE_BUDGETS.maxTelemetryDiagnosticCodes);
332+
expect(forward.diagnosticCodes).toContain('RUSH_OPERATION_FAILED');
333+
expect(forward.diagnosticCodes).toContain('RUSH_DEPENDENCY_TOOL_FAILED');
334+
expect(forward.diagnosticCodes).not.toContain(hostilePrivateCodes[0]);
335+
expect(forward.diagnosticCategoryCounts).toEqual({
336+
other: publicCodes.length + hostilePrivateCodes.length,
337+
operation: 1,
338+
'dependency-tool': 1
339+
});
340+
expect(Object.keys(forward.diagnosticCategoryCounts)).toHaveLength(3);
341+
});
342+
155343
it('projects public envelopes while preserving allowlisted diagnostic fields deterministically', async () => {
156344
const PUBLIC_EXTENSION_SOURCE: IReporterEventSource = {
157345
packageName: '@rushstack/public-reporter-plugin',

0 commit comments

Comments
 (0)