From fa1b3692d19e44c227a25d5b1f8e02928fbf6a04 Mon Sep 17 00:00:00 2001 From: Ankita Dodamani Date: Thu, 23 Jul 2026 13:59:28 +0530 Subject: [PATCH] feat(telemetry): harden redaction, retry failed sends, www endpoint, debug env MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - redaction: scrub KEY=secret credentials (incl. FOO_SECRET=), Bearer tokens, prefixed keys (ghp_/sk-/AKIA/xox…), more URL schemes; + a fuzz test - retry queue: failed sends persisted to ~/.config/chaibuilder/telemetry-queue.jsonl (capped 50) and drained + resent on the next run; transient/5xx retried, 4xx dropped - default endpoint → https://www.chaibuilder.com/in/cli/track (skips the 308 hop) - CHAIBUILDER_TELEMETRY_DEBUG echoes each event to stderr - remove the unused first-run notice dead code Telemetry stays anonymous with env-only opt-out (DO_NOT_TRACK / CHAIBUILDER_TELEMETRY_DISABLED); no notice or subcommand. --- docs/telemetry-enhancements.md | 13 ++++ src/core/real-adapters.ts | 6 +- src/lib/telemetry-notice.ts | 26 -------- src/lib/telemetry-queue.ts | 39 +++++++++++ src/lib/telemetry.ts | 70 ++++++++++++++++---- tests/lib/telemetry-queue.test.ts | 30 +++++++++ tests/lib/telemetry.test.ts | 105 +++++++++++++++++++++++++++++- 7 files changed, 248 insertions(+), 41 deletions(-) delete mode 100644 src/lib/telemetry-notice.ts create mode 100644 src/lib/telemetry-queue.ts create mode 100644 tests/lib/telemetry-queue.test.ts diff --git a/docs/telemetry-enhancements.md b/docs/telemetry-enhancements.md index b039105..fc9b707 100644 --- a/docs/telemetry-enhancements.md +++ b/docs/telemetry-enhancements.md @@ -57,6 +57,19 @@ Possible improvements to the ChaiBuilder CLI telemetry (CLI events, the **No endpoint change required:** `distinct_id`/`run_id` are existing top-level fields (values only changed); `is_ci`, `code`, `duration_ms` ride inside `properties`, which the endpoint forwards to PostHog wholesale. +### Implemented — phase 2 (robustness, endpoint, debug) + +- **2.5 redaction hardening** — key=value credentials (incl. `FOO_SECRET=`), Bearer tokens, prefixed keys (`ghp_`/`sk-`/`AKIA`/`xox…`), more URL schemes; + a fuzz test. +- **3.2 www endpoint** — default is `https://www.chaibuilder.com/in/cli/track` (skips the 308 hop; verified non-www 308→www and www responds directly). +- **3.6 retry queue** — failed sends persist to `telemetry-queue.jsonl` (capped 50) and drain + resend on the next run (transient/5xx only; 4xx dropped). +- **6.1 `CHAIBUILDER_TELEMETRY_DEBUG`** — echoes each event to stderr. + +**Deliberately not done — consent UI (2.1 / 2.2 / 2.3 / 2.4):** telemetry is anonymous, so there is no first-run notice, no `telemetry` subcommand, and no persisted config opt-out. Opt-out is env-only: `DO_NOT_TRACK=1` or `CHAIBUILDER_TELEMETRY_DISABLED=1`. + +**N/A — 1.7 homepage source:** the CLI has no remote homepage fetch (the first page always uses bundled `DEFAULT_BLOCKS`), so there's no remote-vs-fallback to segment. Revisit only if a remote homepage fetch is added. + +**Still not in this repo:** 3.1 batching / 3.3 rate-limit / 3.4 geo (cbpl endpoint), 4.x dashboards (PostHog UI), 5.2/5.4 CI (cli-e2e has no remote), 1.6 extra dimensions (needs a data-scope decision), 3.5 posthog-node (would bypass the server-side endpoint — not recommended). + --- ## 2. Privacy & consent diff --git a/src/core/real-adapters.ts b/src/core/real-adapters.ts index 6b9513e..5bf1bd1 100644 --- a/src/core/real-adapters.ts +++ b/src/core/real-adapters.ts @@ -9,6 +9,7 @@ import path from 'node:path' import { execa } from 'execa' import { downloadTemplate } from 'giget' import { createTelemetry, resolveDistinctId, telemetryDisabled } from '../lib/telemetry.js' +import { telemetryQueuePath } from '../lib/telemetry-queue.js' import type { ExecPort, FsPort, @@ -227,6 +228,7 @@ function createLog(): LoggerPort { } export function createRealPorts(): Ports { + const optedOut = telemetryDisabled() return { prompts: createPrompts(), fs: createFs(), @@ -235,7 +237,9 @@ export function createRealPorts(): Ports { log: createLog(), telemetry: createTelemetry({ cliVersion: cliVersion(), - distinctId: telemetryDisabled() ? undefined : resolveDistinctId(), + disabled: optedOut, + distinctId: optedOut ? undefined : resolveDistinctId(), + queueFile: telemetryQueuePath(), }), randomBytes: (size) => nodeRandomBytes(size), } diff --git a/src/lib/telemetry-notice.ts b/src/lib/telemetry-notice.ts deleted file mode 100644 index 107b76a..0000000 --- a/src/lib/telemetry-notice.ts +++ /dev/null @@ -1,26 +0,0 @@ -import os from 'node:os' -import pc from 'picocolors' -import type { Ports } from '../core/adapters.js' -import { telemetryDisabled } from './telemetry.js' - -/** Show the telemetry notice once per machine (marker in ~/.config/chaibuilder). */ -export async function showTelemetryNoticeOnce(ports: Ports): Promise { - if (telemetryDisabled()) return - - const dir = ports.fs.join(os.homedir(), '.config', 'chaibuilder') - const marker = ports.fs.join(dir, 'telemetry.json') - - try { - if (await ports.fs.exists(marker)) return - } catch {} - - ports.log.message(pc.bold('Help improve ChaiBuilder')) - ports.log.note(pc.dim('We collect anonymous usage data to make this CLI better.')) - ports.log.note(pc.dim('From your device, that\'s just your OS — nothing else')) - ports.log.note(pc.dim('Prefer to opt out? Set DO_NOT_TRACK=1 or CHAIBUILDER_TELEMETRY_DISABLED=1')) - - try { - await ports.fs.mkdir(dir, { recursive: true }) - await ports.fs.writeFile(marker, JSON.stringify({ noticeShownAt: new Date().toISOString() })) - } catch {} -} diff --git a/src/lib/telemetry-queue.ts b/src/lib/telemetry-queue.ts new file mode 100644 index 0000000..cdecb2d --- /dev/null +++ b/src/lib/telemetry-queue.ts @@ -0,0 +1,39 @@ +import { mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { homedir } from 'node:os' +import { dirname, join } from 'node:path' + +const MAX_QUEUED = 50 + +export function telemetryQueuePath(): string { + return join(homedir(), '.config', 'chaibuilder', 'telemetry-queue.jsonl') +} + +function readQueue(file: string): string[] { + try { + return readFileSync(file, 'utf8') + .split('\n') + .filter((l) => l.trim()) + } catch { + return [] + } +} + +/** Append a failed event body, keeping only the most recent MAX_QUEUED. Best-effort. */ +export function enqueueFailed(body: string, file: string = telemetryQueuePath()): void { + try { + const next = [...readQueue(file), body].slice(-MAX_QUEUED) + mkdirSync(dirname(file), { recursive: true }) + writeFileSync(file, next.join('\n') + '\n') + } catch {} +} + +/** Read and remove all queued bodies. Best-effort; returns [] when empty. */ +export function drainQueue(file: string = telemetryQueuePath()): string[] { + const items = readQueue(file) + if (items.length) { + try { + rmSync(file, { force: true }) + } catch {} + } + return items +} diff --git a/src/lib/telemetry.ts b/src/lib/telemetry.ts index 6884bff..df0f2a4 100644 --- a/src/lib/telemetry.ts +++ b/src/lib/telemetry.ts @@ -3,8 +3,9 @@ import { mkdirSync, readFileSync, writeFileSync } from 'node:fs' import { homedir } from 'node:os' import { dirname, join } from 'node:path' import type { TelemetryPort } from '../core/adapters.js' +import { drainQueue, enqueueFailed } from './telemetry-queue.js' -const DEFAULT_ENDPOINT = 'https://chaibuilder.com/in/cli/track' +const DEFAULT_ENDPOINT = 'https://www.chaibuilder.com/in/cli/track' const SEND_TIMEOUT_MS = 2000 export type TelemetryOptions = { @@ -17,6 +18,8 @@ export type TelemetryOptions = { fetchImpl?: typeof fetch /** Persistent anonymous id (per machine). Falls back to a per-run id if omitted. */ distinctId?: string + /** File for the failed-send retry queue. When set, failed sends are queued and drained on init. */ + queueFile?: string } export type StepAction = 'start' | 'finish' @@ -27,6 +30,11 @@ export function telemetryDisabled(): boolean { return Boolean(process.env.CHAIBUILDER_TELEMETRY_DISABLED || process.env.DO_NOT_TRACK) } +/** Debug mode: echo each event to stderr instead of/alongside sending. */ +export function telemetryDebug(): boolean { + return Boolean(process.env.CHAIBUILDER_TELEMETRY_DEBUG) +} + /** `cmd:step:start` for a start, `cmd:step:finish:result` for a finish. */ export function stepEvent(cmd: string, step: string, action: StepAction, result?: StepResult): string { return result ? `${cmd}:${step}:${action}:${result}` : `${cmd}:${step}:${action}` @@ -106,10 +114,21 @@ export function createTelemetry(opts: TelemetryOptions): TelemetryPort { os: process.platform, } const baseProps = { is_ci: isCI() } + const debug = telemetryDebug() + const queueFile = opts.queueFile const inflight = new Set>() + function post(body: string): void { + if (typeof doFetch !== 'function') return + const promise = send(doFetch, endpoint, body).then((result) => { + if (result === 'retry' && queueFile) enqueueFailed(body, queueFile) + }) + inflight.add(promise) + void promise.finally(() => inflight.delete(promise)) + } + function capture(event: string, properties: Record = {}): void { - if (disabled || typeof doFetch !== 'function') return + if (disabled) return let body: string try { body = JSON.stringify({ @@ -121,9 +140,13 @@ export function createTelemetry(opts: TelemetryOptions): TelemetryPort { } catch { return } - const promise = send(doFetch, endpoint, body) - inflight.add(promise) - void promise.finally(() => inflight.delete(promise)) + if (debug) process.stderr.write(`[telemetry] ${body}\n`) + post(body) + } + + // Retry events that failed to send on a previous run. + if (!disabled && queueFile && typeof doFetch === 'function') { + for (const body of drainQueue(queueFile)) post(body) } async function flush(timeoutMs = SEND_TIMEOUT_MS): Promise { @@ -148,7 +171,7 @@ export function errorDetails(err: unknown): Record { const e = err as { name?: string; message?: unknown; constructor?: { name?: string } } return { error_type: e?.name || e?.constructor?.name || 'Error', - error: redact(String(e?.message ?? err)).slice(0, 300), + error: redact(String(e?.message ?? err).slice(0, 2000)).slice(0, 300), } } @@ -156,20 +179,34 @@ export function errorDetails(err: unknown): Record { export function redact(s: string): string { return s .replace(/[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,}/gi, '') - .replace(/\b(?:libsql|https?|wss?|postgres(?:ql)?|file):\/\/\S+/gi, '') + .replace(/\b(?:libsql|https?|wss?|postgres(?:ql)?|mysql|mongodb(?:\+srv)?|redis|amqps?|file):\/\/\S+/gi, '') + .replace(/\bbearer\s+[A-Za-z0-9._~+/=-]+/gi, 'Bearer ') + .replace( + /\b(\w*(?:password|passwd|pwd|secret|token|api[_-]?key|auth[_-]?token|access[_-]?key|client[_-]?secret))(["']?\s*[:=]\s*["']?)([^\s"'&,;]+)/gi, + '$1$2', + ) + .replace(/\b(?:ghp|gho|ghs|ghr|ghu|xox[baprs])[_-][A-Za-z0-9_-]{6,}/g, '') + .replace(/\b(?:sk|pk|rk)[_-][A-Za-z0-9_-]{10,}/g, '') + .replace(/\b(?:AKIA|ASIA)[A-Z0-9]{12,}\b/g, '') .replace(/[A-Za-z]:\\[^\s"']+/g, '') .replace(/(?:\/[^\s/:"']+){2,}/g, '') .replace(/\b[A-Za-z0-9_-]{24,}\b/g, '') } -/** POST one event; never throws, and always settles within SEND_TIMEOUT_MS. */ -async function send(doFetch: typeof fetch, endpoint: string, body: string): Promise { +type SendResult = 'ok' | 'retry' | 'drop' + +/** + * POST one event; never throws, settles within SEND_TIMEOUT_MS. + * `ok` = delivered, `retry` = transient (network/timeout/5xx) so re-queue, + * `drop` = permanent (4xx) so don't re-queue. + */ +async function send(doFetch: typeof fetch, endpoint: string, body: string): Promise { const controller = new AbortController() let timer: ReturnType | undefined - const timeout = new Promise((resolve) => { + const timeout = new Promise((resolve) => { timer = setTimeout(() => { controller.abort() - resolve() + resolve('retry') }, SEND_TIMEOUT_MS) timer.unref?.() }) @@ -178,9 +215,16 @@ async function send(doFetch: typeof fetch, endpoint: string, body: string): Prom headers: { 'content-type': 'application/json' }, body, signal: controller.signal, - }).then(() => undefined, () => undefined) + }).then( + (res): SendResult => { + if (!res || typeof res.status !== 'number') return 'ok' + if (res.ok) return 'ok' + return res.status >= 500 ? 'retry' : 'drop' + }, + (): SendResult => 'retry', + ) try { - await Promise.race([attempt, timeout]) + return await Promise.race([attempt, timeout]) } finally { if (timer) clearTimeout(timer) } diff --git a/tests/lib/telemetry-queue.test.ts b/tests/lib/telemetry-queue.test.ts new file mode 100644 index 0000000..124f4ca --- /dev/null +++ b/tests/lib/telemetry-queue.test.ts @@ -0,0 +1,30 @@ +import { mkdtempSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { describe, expect, it } from 'vitest' +import { drainQueue, enqueueFailed } from '../../src/lib/telemetry-queue.js' + +const tmpQueue = () => join(mkdtempSync(join(tmpdir(), 'chai-q-')), 'queue.jsonl') + +describe('telemetry-queue', () => { + it('enqueue then drain returns items and clears the file', () => { + const f = tmpQueue() + enqueueFailed('{"a":1}', f) + enqueueFailed('{"b":2}', f) + expect(drainQueue(f)).toEqual(['{"a":1}', '{"b":2}']) + expect(drainQueue(f)).toEqual([]) // cleared after drain + }) + + it('caps the queue at 50, keeping the most recent', () => { + const f = tmpQueue() + for (let i = 0; i < 60; i++) enqueueFailed(`{"i":${i}}`, f) + const items = drainQueue(f) + expect(items).toHaveLength(50) + expect(items[0]).toBe('{"i":10}') // oldest 10 dropped + expect(items[49]).toBe('{"i":59}') + }) + + it('draining a missing file is empty', () => { + expect(drainQueue(join(tmpdir(), 'chai-nope-xyz.jsonl'))).toEqual([]) + }) +}) diff --git a/tests/lib/telemetry.test.ts b/tests/lib/telemetry.test.ts index 94c83a4..a9692ab 100644 --- a/tests/lib/telemetry.test.ts +++ b/tests/lib/telemetry.test.ts @@ -10,9 +10,11 @@ import { redact, resolveDistinctId, stepEvent, + telemetryDebug, telemetryDisabled, trackStep, } from '../../src/lib/telemetry.js' +import { drainQueue } from '../../src/lib/telemetry-queue.js' import { createFakeTelemetry } from '../fakes.js' type Call = { url: string; body: any } @@ -32,6 +34,7 @@ describe('telemetry', () => { delete process.env.DO_NOT_TRACK delete process.env.CHAIBUILDER_TELEMETRY_DISABLED delete process.env.CHAIBUILDER_TELEMETRY_URL + delete process.env.CHAIBUILDER_TELEMETRY_DEBUG }) afterEach(() => { for (const k of Object.keys(process.env)) if (!(k in savedEnv)) delete process.env[k] @@ -47,7 +50,7 @@ describe('telemetry', () => { await t.flush() expect(calls).toHaveLength(2) - expect(calls[0].url).toBe('https://chaibuilder.com/in/cli/track') + expect(calls[0].url).toBe('https://www.chaibuilder.com/in/cli/track') expect(calls[0].body).toMatchObject({ event: 'create:template_clone:start', cli_version: '9.9.9', @@ -86,6 +89,36 @@ describe('telemetry', () => { expect(redact('key ABCDEFGHIJKLMNOPQRSTUVWX12')).toContain('') }) + it('redact scrubs credentials, bearer tokens, prefixed keys, and more URL schemes', () => { + expect(redact('PAYLOAD_SECRET=super-secret-value')).not.toContain('super-secret-value') + expect(redact('password: hunter2xyz')).not.toContain('hunter2xyz') + expect(redact('Authorization: Bearer abc.def.ghijkl')).not.toContain('abc.def.ghijkl') + expect(redact('using ghp_ABCDEFGHIJKLMNOP1234')).toContain('') + expect(redact('key sk-live-abcdEFGH1234')).toContain('') + expect(redact('id AKIAIOSFODNN7EXAMPLE denied')).toContain('') + expect(redact('mongodb+srv://u:p@cluster0.mongodb.net/db')).toContain('') + }) + + it('redact leaves benign text intact', () => { + expect(redact('migration failed on table users')).toBe('migration failed on table users') + expect(redact('exit code 1 while running install')).toBe('exit code 1 while running install') + }) + + it('redact never leaks embedded secrets (fuzz)', () => { + const secrets = ['ghp_' + 'a'.repeat(30), 'sk-live-' + 'b'.repeat(24), 'AKIA' + 'C'.repeat(16), 'x'.repeat(40)] + const templates = [ + (s: string) => `connect failed authToken=${s} at /home/u/app.ts`, + (s: string) => `error: password=${s};`, + (s: string) => `open libsql://db.turso.io?token=${s}`, + (s: string) => `Bearer ${s} was rejected`, + ] + for (const s of secrets) { + for (const t of templates) { + expect(redact(t(s))).not.toContain(s) + } + } + }) + it('sends nothing when disabled via option', async () => { const { calls, fetchImpl } = stubFetch() const t = createTelemetry({ cliVersion: '1.0.0', fetchImpl, disabled: true }) @@ -103,6 +136,27 @@ describe('telemetry', () => { expect(telemetryDisabled()).toBe(true) }) + it('telemetryDebug reflects CHAIBUILDER_TELEMETRY_DEBUG', () => { + expect(telemetryDebug()).toBe(false) + process.env.CHAIBUILDER_TELEMETRY_DEBUG = '1' + expect(telemetryDebug()).toBe(true) + }) + + it('echoes events to stderr in debug mode', async () => { + process.env.CHAIBUILDER_TELEMETRY_DEBUG = '1' + const writes: string[] = [] + const spy = vi.spyOn(process.stderr, 'write').mockImplementation(((s: any) => { + writes.push(String(s)) + return true + }) as any) + const { fetchImpl } = stubFetch() + const t = createTelemetry({ cliVersion: '1.0.0', fetchImpl }) + t.capture('create:setup:start', { template: 'x' }) + await t.flush() + spy.mockRestore() + expect(writes.some((w) => w.includes('[telemetry]') && w.includes('create:setup:start'))).toBe(true) + }) + it('respects CHAIBUILDER_TELEMETRY_URL override', async () => { process.env.CHAIBUILDER_TELEMETRY_URL = 'https://example.test/track' const { calls, fetchImpl } = stubFetch() @@ -130,6 +184,55 @@ describe('telemetry', () => { await expect(t.flush()).resolves.toBeUndefined() }) + it('queues a failed send and resends it on the next init', async () => { + const file = join(mkdtempSync(join(tmpdir(), 'chai-tq-')), 'queue.jsonl') + // run 1: fetch always fails → the event is queued + const failing = vi.fn(async () => { + throw new Error('offline') + }) as unknown as typeof fetch + const t1 = createTelemetry({ cliVersion: '1.0.0', fetchImpl: failing, queueFile: file, disabled: false }) + t1.capture('create:setup:start') + await t1.flush() + expect(failing).toHaveBeenCalledTimes(1) + + // run 2: fetch succeeds → the queued event is drained and resent + const ok = stubFetch() + const t2 = createTelemetry({ cliVersion: '1.0.0', fetchImpl: ok.fetchImpl, queueFile: file, disabled: false }) + await t2.flush() + expect(ok.calls.some((c) => c.body.event === 'create:setup:start')).toBe(true) + }) + + it('does not touch the queue when no queueFile is given', async () => { + const failing = vi.fn(async () => { + throw new Error('offline') + }) as unknown as typeof fetch + const t = createTelemetry({ cliVersion: '1.0.0', fetchImpl: failing, disabled: false }) + t.capture('x') + await expect(t.flush()).resolves.toBeUndefined() // no throw, no queue side effects + }) + + it('does not re-queue permanent (4xx) failures', async () => { + const file = join(mkdtempSync(join(tmpdir(), 'chai-tq4-')), 'queue.jsonl') + const fetch400 = vi.fn(async () => new Response(null, { status: 400 })) as unknown as typeof fetch + const t = createTelemetry({ cliVersion: '1.0.0', fetchImpl: fetch400, queueFile: file, disabled: false }) + t.capture('x') + await t.flush() + expect(drainQueue(file)).toEqual([]) // 4xx is permanent — not retried + }) + + it('queues already-redacted bodies (no secret leak into the retry file)', async () => { + const file = join(mkdtempSync(join(tmpdir(), 'chai-tqr-')), 'queue.jsonl') + const failing = vi.fn(async () => { + throw new Error('offline') + }) as unknown as typeof fetch + const t = createTelemetry({ cliVersion: '1.0.0', fetchImpl: failing, queueFile: file, disabled: false }) + t.capture('create:db_migration:finish:error', errorDetails(new Error('boom password=hunter2secret'))) + await t.flush() + const queued = drainQueue(file).join('\n') + expect(queued).not.toContain('hunter2secret') + expect(queued).toContain('') + }) + it('uses the provided persistent distinctId with a separate run_id', async () => { const { calls, fetchImpl } = stubFetch() const t = createTelemetry({ cliVersion: '1.0.0', fetchImpl, distinctId: 'persist-123' })