fix(telemetry): gate unsafe numeric telemetry values (#1478)

- Drop numeric telemetry properties whose absolute value exceeds Number.MAX_SAFE_INTEGER before enqueue, and reject them again during payload assembly as a backstop.

- Omit constrained_memory_bytes when process.constrainedMemory() is not a safe non-negative integer so Linux no-cgroup sentinels do not overflow int64 parsing downstream.

- Add telemetry tests for unsafe numeric properties and constrained_memory_bytes filtering.
This commit is contained in:
7Sageer 2026-07-07 20:32:02 +08:00 committed by GitHub
parent d1a964fba9
commit 2065ae4615
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 105 additions and 6 deletions

View file

@ -79,8 +79,7 @@ export class SystemMetricsCollector {
const mem = process.memoryUsage();
const constrainedMemory = getConstrainedMemoryBytes();
this.client.track(SYSTEM_METRICS_EVENT, {
const properties: TelemetryProperties = {
process_started_at: this.processStartedAtSeconds,
process_uptime_ms: Math.round(process.uptime() * 1000),
rss_bytes: mem.rss,
@ -88,7 +87,6 @@ export class SystemMetricsCollector {
heap_total_bytes: mem.heapTotal,
external_bytes: mem.external,
array_buffers_bytes: mem.arrayBuffers,
constrained_memory_bytes: constrainedMemory,
cpu_user_us: cpu.user,
cpu_system_us: cpu.system,
cpu_elapsed_us: Math.round(elapsedUs),
@ -96,12 +94,17 @@ export class SystemMetricsCollector {
free_mem_bytes: freemem(),
total_mem_bytes: totalmem(),
cpu_count: cpus().length,
});
};
if (constrainedMemory !== undefined) {
properties['constrained_memory_bytes'] = constrainedMemory;
}
this.client.track(SYSTEM_METRICS_EVENT, properties);
}
}
function getConstrainedMemoryBytes(): number | undefined {
if (typeof process.constrainedMemory !== 'function') return undefined;
const value = process.constrainedMemory();
return Number.isFinite(value) ? value : undefined;
return Number.isSafeInteger(value) && value >= 0 ? value : undefined;
}

View file

@ -2,6 +2,8 @@ export type TelemetryPrimitive = boolean | number | string | undefined | null;
export type TelemetryProperties = Record<string, TelemetryPrimitive>;
export type TelemetryContext = Record<string, TelemetryPrimitive>;
const MAX_TELEMETRY_NUMBER_MAGNITUDE = Number.MAX_SAFE_INTEGER;
export interface TelemetryEvent {
readonly event_id: string;
device_id: string | null;
@ -27,6 +29,10 @@ export function isTelemetryPrimitive(value: unknown): value is TelemetryPrimitiv
value === undefined ||
typeof value === 'boolean' ||
typeof value === 'string' ||
(typeof value === 'number' && Number.isFinite(value))
(typeof value === 'number' && isTelemetryNumber(value))
);
}
function isTelemetryNumber(value: number): boolean {
return Number.isFinite(value) && Math.abs(value) <= MAX_TELEMETRY_NUMBER_MAGNITUDE;
}

View file

@ -160,6 +160,21 @@ describe('TelemetryClient', () => {
expect(transport.sent).toHaveLength(0);
});
it('drops unsafe numeric properties before enqueueing events', async () => {
const client = new TelemetryClient();
const transport = new RecordingTransport();
client.attachSink(makeSink(transport));
client.track('big_number', { big: 2 ** 64, keep: true });
await client.flush();
const event = transport.sent[0]?.[0];
if (event === undefined) throw new Error('Expected a telemetry event');
expect(event.event).toBe('big_number');
expect(event.properties).not.toHaveProperty('big');
expect(event.properties['keep']).toBe(true);
});
it('stops the previous system metrics collector when replacing it', () => {
const client = new TelemetryClient();
const first = { stop: vi.fn() };
@ -267,6 +282,70 @@ describe('SystemMetricsCollector', () => {
expect(numberProperty(event.properties, 'cpu_count')).toBeGreaterThanOrEqual(1);
});
it('omits constrained_memory_bytes when it is not a safe non-negative integer', () => {
vi.useFakeTimers();
vi.spyOn(process, 'constrainedMemory').mockReturnValue(2 ** 64);
const tracked: Array<{
event: string;
properties: Record<string, number | string | boolean | undefined | null>;
}> = [];
const client = {
track(
event: string,
properties: Record<string, number | string | boolean | undefined | null> = {},
): void {
tracked.push({ event, properties });
},
};
const collector = new SystemMetricsCollector({
client,
intervalMs: 30_000,
warmupSampleMs: 1_500,
});
collector.start();
vi.advanceTimersByTime(1_500);
collector.stop();
expect(tracked).toHaveLength(1);
const event = tracked[0];
if (event === undefined) throw new Error('Expected a system_metrics event');
expect(event.event).toBe('system_metrics');
expect(event.properties).not.toHaveProperty('constrained_memory_bytes');
expect(numberProperty(event.properties, 'rss_bytes')).toBeGreaterThan(0);
});
it('reports constrained_memory_bytes when it is a safe non-negative integer', () => {
vi.useFakeTimers();
vi.spyOn(process, 'constrainedMemory').mockReturnValue(8 * 1024 ** 3);
const tracked: Array<{
event: string;
properties: Record<string, number | string | boolean | undefined | null>;
}> = [];
const client = {
track(
event: string,
properties: Record<string, number | string | boolean | undefined | null> = {},
): void {
tracked.push({ event, properties });
},
};
const collector = new SystemMetricsCollector({
client,
intervalMs: 30_000,
warmupSampleMs: 1_500,
});
collector.start();
vi.advanceTimersByTime(1_500);
collector.stop();
expect(tracked).toHaveLength(1);
const event = tracked[0];
if (event === undefined) throw new Error('Expected a system_metrics event');
expect(event.properties['constrained_memory_bytes']).toBe(8 * 1024 ** 3);
});
it('does not duplicate interval sampling when started twice', () => {
vi.useFakeTimers();
const tracked: string[] = [];
@ -363,6 +442,17 @@ describe('payload assembly', () => {
expect(() => buildPayload([event], 'device-1')).toThrow(/property.nested/);
});
it('rejects unsafe numeric property values before outbound send', () => {
const event = {
...sampleEvent('bad_number'),
properties: {
big: 2 ** 64,
},
};
expect(() => buildPayload([event], 'device-1')).toThrow(/property.big/);
});
it('rejects nested context and array property values before outbound send', () => {
const nestedContext = {
...sampleEvent('bad_context'),