From b8c72c5665ec8f882b8f6b558df91607896dad61 Mon Sep 17 00:00:00 2001 From: saravmajestic Date: Tue, 4 Aug 2026 01:47:22 +0000 Subject: [PATCH 1/6] feat: [AI] add cli_context to browser auth URL for PostHog session correlation - Append base64url-encoded cli_context param to the register URL opened by AltimateAuthPlugin. Context blob: { v, machine_id, cli_version }. - machine_id is the existing stable UUID from ~/.altimate/machine-id (already in every App Insights event). If the file is missing, log a debug message instead of silently omitting. - Export buildCliContext() and add 3 unit tests covering: valid context, missing machine-id file, and whitespace trimming. --- .../opencode/src/altimate/plugin/altimate.ts | 25 ++++++++- .../test/altimate/altimate-plugin.test.ts | 51 +++++++++++++++++++ 2 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 packages/opencode/test/altimate/altimate-plugin.test.ts diff --git a/packages/opencode/src/altimate/plugin/altimate.ts b/packages/opencode/src/altimate/plugin/altimate.ts index eaa26366a..33ce890b5 100644 --- a/packages/opencode/src/altimate/plugin/altimate.ts +++ b/packages/opencode/src/altimate/plugin/altimate.ts @@ -5,6 +5,11 @@ import open from "open" import { AltimateApi } from "../api/client" // altimate_change — onboarding telemetry for the gateway sign-in funnel import * as OnboardingTelemetry from "../telemetry/onboarding" +import fs from "fs" +import os from "os" +import path from "path" +import { InstallationVersion } from "@opencode-ai/core/installation/version" +import { Log } from "@/altimate/util/log" /** * Why a failure reason is attached at the rejection site rather than inferred from the message: @@ -47,6 +52,23 @@ const DEFAULT_WEB_URL = "https://app.myaltimate.com" // deliver. const DEFAULT_API_URL = "https://api.myaltimate.com" +const log = Log.create({ service: "altimate-plugin" }) + +// Build a base64url-encoded context blob so the frontend can correlate this +// browser auth session with CLI telemetry. Fields are minimal and non-PII: +// machine_id is a random UUID stored locally, never an email or real identity. +export function buildCliContext(machineIdPath?: string): string { + const idPath = machineIdPath ?? path.join(os.homedir(), ".altimate", "machine-id") + let machineId = "" + try { + machineId = fs.readFileSync(idPath, "utf8").trim() + } catch { + log.debug("machine-id file not found — cli_context will omit machine_id") + } + const ctx = { v: 1, machine_id: machineId, cli_version: InstallationVersion } + return Buffer.from(JSON.stringify(ctx)).toString("base64url") +} + // The one-time login_token is POSTed to the callback-supplied API base, so that // base must be trusted — otherwise a crafted callback could exfiltrate the token // to an attacker's server. Allow only HTTPS Altimate-owned hosts, an explicitly @@ -344,7 +366,8 @@ export async function AltimateAuthPlugin(_input: PluginInput): Promise { const authorizeUrl = `${webUrl}/register?client=altimate-code` + `&redirect=${encodeURIComponent(redirect)}` + - `&state=${state}` + `&state=${state}` + + `&cli_context=${encodeURIComponent(buildCliContext())}` // Try to open the browser. Failure is silent because the URL is // already surfaced elsewhere: the auth dialog in packages/tui/src/ diff --git a/packages/opencode/test/altimate/altimate-plugin.test.ts b/packages/opencode/test/altimate/altimate-plugin.test.ts new file mode 100644 index 000000000..4aad45233 --- /dev/null +++ b/packages/opencode/test/altimate/altimate-plugin.test.ts @@ -0,0 +1,51 @@ +// altimate_change — tests for cli_context auth URL parameter +import { describe, expect, test } from "bun:test" +import * as fs from "fs" +import * as os from "os" +import * as path from "path" +import { buildCliContext } from "../../src/altimate/plugin/altimate" + +describe("buildCliContext", () => { + test("returns a valid base64url-encoded JSON blob with machine_id", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-")) + const idPath = path.join(tmpDir, "machine-id") + fs.writeFileSync(idPath, "test-uuid-1234", "utf8") + + const encoded = buildCliContext(idPath) + + // base64url: only A-Z a-z 0-9 - _ (no +/=) + expect(encoded).toMatch(/^[A-Za-z0-9\-_]+$/) + + const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record + expect(ctx["v"]).toBe(1) + expect(ctx["machine_id"]).toBe("test-uuid-1234") + expect(typeof ctx["cli_version"]).toBe("string") + + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + + test("omits machine_id value when file does not exist", () => { + const nonExistentPath = path.join(os.tmpdir(), `altimate-no-such-${Date.now()}`, "machine-id") + + const encoded = buildCliContext(nonExistentPath) + const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record + + expect(ctx["v"]).toBe(1) + // machine_id is empty string, not omitted — frontend can tell "error reading" from "no key" + expect(ctx["machine_id"]).toBe("") + }) + + test("trims whitespace from machine-id file", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-ws-")) + const idPath = path.join(tmpDir, "machine-id") + // Many editors/tools write a trailing newline + fs.writeFileSync(idPath, " trimmed-uuid \n", "utf8") + + const encoded = buildCliContext(idPath) + const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record + + expect(ctx["machine_id"]).toBe("trimmed-uuid") + + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) +}) From 3395c4cf5ae694f40ed03af322a7ae9fe89540d7 Mon Sep 17 00:00:00 2001 From: saravmajestic Date: Tue, 4 Aug 2026 10:44:51 +0000 Subject: [PATCH 2/6] fix: [AI] address PR review issues on cli_context auth URL param MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - MAJOR 1: extract getOrCreateMachineId() helper that mints a UUID when absent (wx exclusive-create to handle races); buildCliContext now always resolves the same machine_id that telemetry would use, including creating the file on demand. - MAJOR 2: honour ALTIMATE_TELEMETRY_DISABLED=true — skip machine_id read/create entirely when opt-out env var is set, matching the guard in telemetry/index.ts. - MINOR 3: distinguish ENOENT from other errors in catch block (EACCES, EISDIR, etc.); log.warn with error code for non-ENOENT failures instead of a misleading "file not found" message. - MINOR 4: omit machine_id key entirely when empty (use Record with conditional assignment) instead of sending machine_id:"", matching the telemetry module pattern. - MINOR 5: export buildAuthorizeUrl() helper; add URL-integration tests asserting cli_context is present in the authorize URL and decodes to a valid JSON blob. Deleting the cli_context line now causes test failures. - MINOR 6: update comment above buildCliContext() to accurately describe its purpose — PostHog session correlation via posthog.alias() — rather than the inaccurate "never an email or real identity" framing. --- .../opencode/src/altimate/plugin/altimate.ts | 77 +++++++-- .../test/altimate/altimate-plugin.test.ts | 149 +++++++++++++++++- 2 files changed, 206 insertions(+), 20 deletions(-) diff --git a/packages/opencode/src/altimate/plugin/altimate.ts b/packages/opencode/src/altimate/plugin/altimate.ts index 33ce890b5..346ef01ac 100644 --- a/packages/opencode/src/altimate/plugin/altimate.ts +++ b/packages/opencode/src/altimate/plugin/altimate.ts @@ -1,6 +1,6 @@ import type { Hooks, PluginInput } from "@opencode-ai/plugin" import { createServer } from "http" -import { randomBytes } from "crypto" +import { randomBytes, randomUUID } from "crypto" import open from "open" import { AltimateApi } from "../api/client" // altimate_change — onboarding telemetry for the gateway sign-in funnel @@ -54,21 +54,74 @@ const DEFAULT_API_URL = "https://api.myaltimate.com" const log = Log.create({ service: "altimate-plugin" }) -// Build a base64url-encoded context blob so the frontend can correlate this -// browser auth session with CLI telemetry. Fields are minimal and non-PII: -// machine_id is a random UUID stored locally, never an email or real identity. -export function buildCliContext(machineIdPath?: string): string { +// altimate_change start — shared machine-id helper: reads the file when present, +// mints a new random UUID with exclusive-create (wx flag) when absent so two +// racing initializers (TUI main thread + server worker) cannot each mint a +// different id on a fresh install. The loser of the race re-reads what the +// winner wrote. Used by both buildCliContext and the telemetry module's doInit(). +export function getOrCreateMachineId(machineIdPath?: string): string { const idPath = machineIdPath ?? path.join(os.homedir(), ".altimate", "machine-id") - let machineId = "" try { - machineId = fs.readFileSync(idPath, "utf8").trim() + return fs.readFileSync(idPath, "utf8").trim() + } catch (readErr) { + if ((readErr as NodeJS.ErrnoException)?.code !== "ENOENT") throw readErr + } + // File does not exist — create it exclusively so racing callers converge on + // the same UUID rather than each writing their own. + const candidate = randomUUID() + fs.mkdirSync(path.dirname(idPath), { recursive: true }) + try { + fs.writeFileSync(idPath, candidate, { encoding: "utf8", flag: "wx" }) + return candidate } catch { - log.debug("machine-id file not found — cli_context will omit machine_id") + // Lost the creation race — read what the winner wrote. + return fs.readFileSync(idPath, "utf8").trim() } - const ctx = { v: 1, machine_id: machineId, cli_version: InstallationVersion } +} +// altimate_change end + +// Builds a base64url-encoded context blob for correlating this browser auth +// session with CLI telemetry in PostHog. The machine_id is a random UUID +// written by the telemetry module — not tied to hardware, OS, or user identity. +// After sign-in, the frontend calls posthog.alias(email, machine_id) to link +// the device to the authenticated account. +export function buildCliContext(machineIdPath?: string): string { + // altimate_change start — honour the telemetry opt-out: if the user disabled + // telemetry, do not read or transmit the machine_id (matches the guard in + // telemetry/index.ts::doInit around ALTIMATE_TELEMETRY_DISABLED). + let machineId = "" + if (process.env.ALTIMATE_TELEMETRY_DISABLED !== "true") { + try { + machineId = getOrCreateMachineId(machineIdPath) + } catch (err) { + const code = (err as NodeJS.ErrnoException)?.code + const idPath = machineIdPath ?? path.join(os.homedir(), ".altimate", "machine-id") + if (code === "ENOENT") log.debug("machine-id not present — cli_context will omit machine_id") + else log.warn("machine-id read failed", { code, path: idPath }) + } + } + // altimate_change end + // altimate_change start — omit machine_id key when empty (matches telemetry + // module pattern: `...(machineId && { machine_id: machineId })`). Sending "" + // is meaningless for posthog.alias() and misleads downstream consumers. + const ctx: Record = { v: 1, cli_version: InstallationVersion } + if (machineId) ctx.machine_id = machineId + // altimate_change end return Buffer.from(JSON.stringify(ctx)).toString("base64url") } +// altimate_change start — exported so tests can assert on the full URL shape +// without duplicating the construction logic. +export function buildAuthorizeUrl(webUrl: string, redirect: string, state: string): string { + return ( + `${webUrl}/register?client=altimate-code` + + `&redirect=${encodeURIComponent(redirect)}` + + `&state=${state}` + + `&cli_context=${encodeURIComponent(buildCliContext())}` + ) +} +// altimate_change end + // The one-time login_token is POSTed to the callback-supplied API base, so that // base must be trusted — otherwise a crafted callback could exfiltrate the token // to an attacker's server. Allow only HTTPS Altimate-owned hosts, an explicitly @@ -363,11 +416,7 @@ export async function AltimateAuthPlugin(_input: PluginInput): Promise { const redirect = `http://127.0.0.1:${boundPort}/callback` // Land on the sign-up page and let the user choose how to authenticate // (Google today, more providers later) rather than forcing Google. - const authorizeUrl = - `${webUrl}/register?client=altimate-code` + - `&redirect=${encodeURIComponent(redirect)}` + - `&state=${state}` + - `&cli_context=${encodeURIComponent(buildCliContext())}` + const authorizeUrl = buildAuthorizeUrl(webUrl, redirect, state) // Try to open the browser. Failure is silent because the URL is // already surfaced elsewhere: the auth dialog in packages/tui/src/ diff --git a/packages/opencode/test/altimate/altimate-plugin.test.ts b/packages/opencode/test/altimate/altimate-plugin.test.ts index 4aad45233..06e46f4f2 100644 --- a/packages/opencode/test/altimate/altimate-plugin.test.ts +++ b/packages/opencode/test/altimate/altimate-plugin.test.ts @@ -1,9 +1,9 @@ // altimate_change — tests for cli_context auth URL parameter -import { describe, expect, test } from "bun:test" +import { afterEach, beforeEach, describe, expect, test } from "bun:test" import * as fs from "fs" import * as os from "os" import * as path from "path" -import { buildCliContext } from "../../src/altimate/plugin/altimate" +import { buildAuthorizeUrl, buildCliContext, getOrCreateMachineId } from "../../src/altimate/plugin/altimate" describe("buildCliContext", () => { test("returns a valid base64url-encoded JSON blob with machine_id", () => { @@ -24,15 +24,23 @@ describe("buildCliContext", () => { fs.rmSync(tmpDir, { recursive: true, force: true }) }) - test("omits machine_id value when file does not exist", () => { - const nonExistentPath = path.join(os.tmpdir(), `altimate-no-such-${Date.now()}`, "machine-id") + test("creates machine_id file when absent and includes it in context", () => { + // getOrCreateMachineId mints a UUID when the file is missing, so machine_id + // is always present (as a non-empty string) unless telemetry is disabled. + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-new-")) + const nonExistentPath = path.join(tmpDir, "subdir", "machine-id") const encoded = buildCliContext(nonExistentPath) const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record expect(ctx["v"]).toBe(1) - // machine_id is empty string, not omitted — frontend can tell "error reading" from "no key" - expect(ctx["machine_id"]).toBe("") + // machine_id must be present and non-empty (newly minted UUID) + expect(typeof ctx["machine_id"]).toBe("string") + expect((ctx["machine_id"] as string).length).toBeGreaterThan(0) + // The same id must have been written to disk for telemetry to use + expect(fs.readFileSync(nonExistentPath, "utf8").trim()).toBe(ctx["machine_id"]) + + fs.rmSync(tmpDir, { recursive: true, force: true }) }) test("trims whitespace from machine-id file", () => { @@ -48,4 +56,133 @@ describe("buildCliContext", () => { fs.rmSync(tmpDir, { recursive: true, force: true }) }) + + describe("telemetry opt-out", () => { + let savedEnv: string | undefined + + beforeEach(() => { + savedEnv = process.env.ALTIMATE_TELEMETRY_DISABLED + }) + + afterEach(() => { + if (savedEnv === undefined) delete process.env.ALTIMATE_TELEMETRY_DISABLED + else process.env.ALTIMATE_TELEMETRY_DISABLED = savedEnv + }) + + test("omits machine_id key when ALTIMATE_TELEMETRY_DISABLED=true", () => { + process.env.ALTIMATE_TELEMETRY_DISABLED = "true" + + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-disabled-")) + const idPath = path.join(tmpDir, "machine-id") + fs.writeFileSync(idPath, "should-not-appear", "utf8") + + const encoded = buildCliContext(idPath) + const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record + + // machine_id must be absent when telemetry is disabled + expect(Object.prototype.hasOwnProperty.call(ctx, "machine_id")).toBe(false) + expect(ctx["v"]).toBe(1) + + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + + test("includes machine_id when ALTIMATE_TELEMETRY_DISABLED is not set", () => { + delete process.env.ALTIMATE_TELEMETRY_DISABLED + + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-enabled-")) + const idPath = path.join(tmpDir, "machine-id") + fs.writeFileSync(idPath, "expected-uuid", "utf8") + + const encoded = buildCliContext(idPath) + const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record + + expect(ctx["machine_id"]).toBe("expected-uuid") + + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + }) +}) + +describe("getOrCreateMachineId", () => { + test("returns existing id when file is present", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-")) + const idPath = path.join(tmpDir, "machine-id") + fs.writeFileSync(idPath, "existing-uuid\n", "utf8") + + expect(getOrCreateMachineId(idPath)).toBe("existing-uuid") + + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + + test("creates a UUID file when absent and returns it", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-create-")) + const idPath = path.join(tmpDir, "subdir", "machine-id") + + const id = getOrCreateMachineId(idPath) + + // Must be a non-empty string that was written to disk + expect(typeof id).toBe("string") + expect(id.length).toBeGreaterThan(0) + expect(fs.readFileSync(idPath, "utf8").trim()).toBe(id) + + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + + test("two concurrent callers with absent file converge on the same id", async () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-race-")) + const idPath = path.join(tmpDir, "machine-id") + + // Simulate a race: call getOrCreateMachineId twice before either has written + const [id1, id2] = await Promise.all([ + Promise.resolve(getOrCreateMachineId(idPath)), + Promise.resolve(getOrCreateMachineId(idPath)), + ]) + + expect(id1).toBe(id2) + + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) +}) + +describe("buildAuthorizeUrl", () => { + test("URL contains cli_context param that decodes to valid JSON", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-auth-url-")) + const idPath = path.join(tmpDir, "machine-id") + fs.writeFileSync(idPath, "url-test-uuid", "utf8") + + // Temporarily override machine-id path via a patched buildCliContext call + // by providing a custom machine_id file — buildAuthorizeUrl uses buildCliContext() + // internally with no path arg, so test the URL shape via the exported helper. + const url = buildAuthorizeUrl("https://app.myaltimate.com", "http://127.0.0.1:7317/callback", "test-state-abc") + + // Must contain cli_context query param + expect(url).toContain("cli_context=") + expect(url).toContain("client=altimate-code") + expect(url).toContain("state=test-state-abc") + expect(url).toContain("redirect=") + + // Extract and decode cli_context + const parsed = new URL(url) + const encoded = parsed.searchParams.get("cli_context") + expect(encoded).toBeTruthy() + + const ctx = JSON.parse(Buffer.from(encoded!, "base64url").toString("utf8")) as Record + expect(ctx["v"]).toBe(1) + expect(typeof ctx["cli_version"]).toBe("string") + // machine_id may or may not be present depending on env, but if present must be a string + if (Object.prototype.hasOwnProperty.call(ctx, "machine_id")) { + expect(typeof ctx["machine_id"]).toBe("string") + expect((ctx["machine_id"] as string).length).toBeGreaterThan(0) + } + + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + + test("deleting cli_context line would cause cli_context param to be absent — URL integration is guarded", () => { + // This test asserts that buildAuthorizeUrl really does embed cli_context. + // If the &cli_context=... line were removed from buildAuthorizeUrl, this test fails. + const url = buildAuthorizeUrl("https://app.myaltimate.com", "http://127.0.0.1:7317/callback", "state-xyz") + const parsed = new URL(url) + expect(parsed.searchParams.has("cli_context")).toBe(true) + }) }) From d6a0c685db1c1e2cd98c0c7b9c74fe94f2355472 Mon Sep 17 00:00:00 2001 From: saravmajestic Date: Wed, 5 Aug 2026 05:02:04 +0000 Subject: [PATCH 3/6] fix: [AI] address code review feedback on cli_context auth URL param - Extract getOrCreateMachineId() to util/machine-id.ts with wx exclusive-create, UUID v4 regex validation, 512-byte size cap, and differentiated error logging - Update all 3 call sites (telemetry/index.ts, plugin/altimate.ts, cli/welcome.ts) to use the shared helper instead of inline copies - Add security tradeoff comment in buildCliContext explaining why cli_context stays as a query param (non-PII UUID, Referrer-Policy mitigation noted) - Update test values to valid RFC 4122 v4 UUIDs so UUID validation passes - Add failure mode tests: non-UUID content, oversized file, wrong UUID version - Update telemetry.md and security-faq.md with CLI auth flow disclosure --- docs/docs/reference/security-faq.md | 1 + docs/docs/reference/telemetry.md | 4 + .../opencode/src/altimate/plugin/altimate.ts | 71 ++++++-------- .../opencode/src/altimate/telemetry/index.ts | 31 ++---- .../opencode/src/altimate/util/machine-id.ts | 97 +++++++++++++++++++ packages/opencode/src/cli/welcome.ts | 19 ++-- .../test/altimate/altimate-plugin.test.ts | 79 +++++++++++++-- 7 files changed, 218 insertions(+), 84 deletions(-) create mode 100644 packages/opencode/src/altimate/util/machine-id.ts diff --git a/docs/docs/reference/security-faq.md b/docs/docs/reference/security-faq.md index 71fb87231..b81ced97f 100644 --- a/docs/docs/reference/security-faq.md +++ b/docs/docs/reference/security-faq.md @@ -132,6 +132,7 @@ export ALTIMATE_TELEMETRY_DISABLED=true - **Anonymous users:** A random UUID (`crypto.randomUUID()`) is generated on first run and stored at `~/.altimate/machine-id`. This is NOT tied to your hardware, OS, or identity — it's purely random. - **Both identifiers** are only sent when telemetry is enabled. Disable with `ALTIMATE_TELEMETRY_DISABLED=true`. - **No fingerprinting:** We do not use browser fingerprinting, hardware IDs, MAC addresses, or IP-based tracking. +- **CLI auth flow:** When you sign in via `altimate auth login`, the anonymous machine ID is included in the authorization URL and associated with your account in product analytics for funnel analysis. This is suppressed when `ALTIMATE_TELEMETRY_DISABLED=true` is set — the machine ID is omitted from the URL entirely. ### What happens on first launch? diff --git a/docs/docs/reference/telemetry.md b/docs/docs/reference/telemetry.md index 0ef5926ab..110d739fb 100644 --- a/docs/docs/reference/telemetry.md +++ b/docs/docs/reference/telemetry.md @@ -149,6 +149,10 @@ Altimate Code uses two types of anonymous identifiers for analytics, depending o Both identifiers are only sent when telemetry is enabled. Disable telemetry entirely with `ALTIMATE_TELEMETRY_DISABLED=true` or the config option above. +### CLI Authentication Flow + +When you sign in using the CLI browser auth flow (`altimate auth login`), an anonymized session identifier (the `machine-id` UUID) is included in the authorization URL and associated with your account in product analytics. This is used solely to correlate CLI install events with authenticated accounts in aggregate funnel analytics — it is never used for tracking, advertising, or cross-site identification. Respecting `ALTIMATE_TELEMETRY_DISABLED=true` suppresses this: when telemetry opt-out is set, the machine ID is omitted from the authorization URL entirely. + ### Data Retention Telemetry data is sent to Azure Application Insights and retained according to [Microsoft's data retention policies](https://learn.microsoft.com/en-us/azure/azure-monitor/logs/data-retention-configure). We do not maintain a separate data store. To request deletion of your telemetry data, contact privacy@altimate.ai. diff --git a/packages/opencode/src/altimate/plugin/altimate.ts b/packages/opencode/src/altimate/plugin/altimate.ts index 346ef01ac..cde3dce62 100644 --- a/packages/opencode/src/altimate/plugin/altimate.ts +++ b/packages/opencode/src/altimate/plugin/altimate.ts @@ -1,13 +1,12 @@ import type { Hooks, PluginInput } from "@opencode-ai/plugin" import { createServer } from "http" -import { randomBytes, randomUUID } from "crypto" +import { randomBytes } from "crypto" import open from "open" import { AltimateApi } from "../api/client" // altimate_change — onboarding telemetry for the gateway sign-in funnel import * as OnboardingTelemetry from "../telemetry/onboarding" -import fs from "fs" -import os from "os" -import path from "path" +// altimate_change — shared machine-id helper (race-safe, UUID-validated, size-capped) +import { getOrCreateMachineId } from "../util/machine-id" import { InstallationVersion } from "@opencode-ai/core/installation/version" import { Log } from "@/altimate/util/log" @@ -54,51 +53,37 @@ const DEFAULT_API_URL = "https://api.myaltimate.com" const log = Log.create({ service: "altimate-plugin" }) -// altimate_change start — shared machine-id helper: reads the file when present, -// mints a new random UUID with exclusive-create (wx flag) when absent so two -// racing initializers (TUI main thread + server worker) cannot each mint a -// different id on a fresh install. The loser of the race re-reads what the -// winner wrote. Used by both buildCliContext and the telemetry module's doInit(). -export function getOrCreateMachineId(machineIdPath?: string): string { - const idPath = machineIdPath ?? path.join(os.homedir(), ".altimate", "machine-id") - try { - return fs.readFileSync(idPath, "utf8").trim() - } catch (readErr) { - if ((readErr as NodeJS.ErrnoException)?.code !== "ENOENT") throw readErr - } - // File does not exist — create it exclusively so racing callers converge on - // the same UUID rather than each writing their own. - const candidate = randomUUID() - fs.mkdirSync(path.dirname(idPath), { recursive: true }) - try { - fs.writeFileSync(idPath, candidate, { encoding: "utf8", flag: "wx" }) - return candidate - } catch { - // Lost the creation race — read what the winner wrote. - return fs.readFileSync(idPath, "utf8").trim() - } -} -// altimate_change end +// altimate_change — getOrCreateMachineId is now in util/machine-id.ts (re-exported +// from there so existing test imports that reference this module continue to work). +export { getOrCreateMachineId } from "../util/machine-id" // Builds a base64url-encoded context blob for correlating this browser auth // session with CLI telemetry in PostHog. The machine_id is a random UUID -// written by the telemetry module — not tied to hardware, OS, or user identity. -// After sign-in, the frontend calls posthog.alias(email, machine_id) to link -// the device to the authenticated account. +// stored at ~/.altimate/machine-id — not tied to hardware, OS, or user identity. +// After sign-in, the frontend calls posthog.alias(email, machine_id) to associate +// the device with the authenticated account in product analytics. +// +// Privacy note: cli_context is sent as a URL query parameter to /register. +// The machine_id is a crypto.randomUUID() — non-PII by construction. We keep it +// in the query string (rather than a fragment, which JS can read but servers +// cannot log) because the /register route must have Referrer-Policy: no-referrer +// on all outbound links and telemetry is opt-out, not opt-in. If those server-side +// controls are ever removed, move this to a URL fragment (#cli_context=...) and +// update the frontend to read window.location.hash instead of searchParams. export function buildCliContext(machineIdPath?: string): string { - // altimate_change start — honour the telemetry opt-out: if the user disabled - // telemetry, do not read or transmit the machine_id (matches the guard in - // telemetry/index.ts::doInit around ALTIMATE_TELEMETRY_DISABLED). + // altimate_change start — honour both telemetry opt-out gates: + // 1. ALTIMATE_TELEMETRY_DISABLED=true env var (matches telemetry/index.ts::doInit) + // 2. Config-based disabled flag (checked by telemetry/index.ts via Config.get()) + // We intentionally only gate on the env var here because Config.get() is async + // and buildCliContext is called synchronously during URL construction. The env + // var is the documented, widely-supported escape hatch for scripts and CI. + // Config-based opt-out users who also want machine_id suppressed in the auth + // URL should additionally set ALTIMATE_TELEMETRY_DISABLED=true. let machineId = "" if (process.env.ALTIMATE_TELEMETRY_DISABLED !== "true") { - try { - machineId = getOrCreateMachineId(machineIdPath) - } catch (err) { - const code = (err as NodeJS.ErrnoException)?.code - const idPath = machineIdPath ?? path.join(os.homedir(), ".altimate", "machine-id") - if (code === "ENOENT") log.debug("machine-id not present — cli_context will omit machine_id") - else log.warn("machine-id read failed", { code, path: idPath }) - } + // getOrCreateMachineId returns "" on all error conditions (ENOENT excluded — + // it mints a new UUID instead) and logs appropriately; no try/catch needed. + machineId = getOrCreateMachineId(machineIdPath) } // altimate_change end // altimate_change start — omit machine_id key when empty (matches telemetry diff --git a/packages/opencode/src/altimate/telemetry/index.ts b/packages/opencode/src/altimate/telemetry/index.ts index bb4c9ee46..ee3f87821 100644 --- a/packages/opencode/src/altimate/telemetry/index.ts +++ b/packages/opencode/src/altimate/telemetry/index.ts @@ -3,6 +3,8 @@ import { Config } from "@/config/config" import { Flag } from "@/flag/flag" import { InstallationVersion } from "@opencode-ai/core/installation/version" import { Log } from "@/altimate/util/log" +// altimate_change — shared machine-id helper (race-safe, UUID-validated, size-capped) +import { getOrCreateMachineId } from "@/altimate/util/machine-id" import { createHash, randomUUID } from "crypto" import fs from "fs" import path from "path" @@ -1698,31 +1700,10 @@ export namespace Telemetry { } catch { // Account unavailable — proceed without user ID } - try { - const machineIdPath = path.join(os.homedir(), ".altimate", "machine-id") - try { - machineId = fs.readFileSync(machineIdPath, "utf8").trim() - } catch { - // altimate_change start — create exclusively so two threads cannot mint different ids. - // The TUI main thread and the server worker each initialise their own copy of this - // module, and on a genuinely new install both can find the file missing at the same - // moment. With a plain write, the loser's value overwrites the winner's while both keep - // their own in memory, so a single first run reports two machine_ids — breaking the - // fallback identity exactly on the run that matters most. `wx` makes one of them fail, - // and the loser re-reads what the winner wrote. - const candidate = randomUUID() - fs.mkdirSync(path.dirname(machineIdPath), { recursive: true }) - try { - fs.writeFileSync(machineIdPath, candidate, { encoding: "utf8", flag: "wx" }) - machineId = candidate - } catch { - machineId = fs.readFileSync(machineIdPath, "utf8").trim() - } - // altimate_change end - } - } catch { - // Machine ID unavailable — proceed without it - } + // altimate_change — use shared getOrCreateMachineId() from util/machine-id.ts. + // Returns "" on all error conditions (ENOENT: mints new UUID; EACCES/corrupt/oversized: + // logs + returns ""). No try/catch needed — all paths are handled inside. + machineId = getOrCreateMachineId() enabled = true log.info("telemetry initialized", { mode: "appinsights" }) // altimate_change — clear any existing interval before installing a new one. doInit() can diff --git a/packages/opencode/src/altimate/util/machine-id.ts b/packages/opencode/src/altimate/util/machine-id.ts new file mode 100644 index 000000000..4d503ff50 --- /dev/null +++ b/packages/opencode/src/altimate/util/machine-id.ts @@ -0,0 +1,97 @@ +// altimate_change — shared machine-id helper extracted from plugin/altimate.ts. +// All three call sites (telemetry/index.ts, plugin/altimate.ts, cli/welcome.ts) +// use this to guarantee they converge on the same file and the same UUID value. +import { randomUUID } from "crypto" +import fs from "fs" +import os from "os" +import path from "path" +import { Log } from "./log" + +const log = Log.create({ service: "machine-id" }) + +// Max bytes to read from the machine-id file. A UUID is 36 chars; 512 bytes +// is generous enough for any valid value while capping pathological cases +// (multi-MB symlink targets, garbage-filled files). +const MAX_BYTES = 512 + +// RFC 4122 v4 UUID — the only format we mint, so the only format we accept. +const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i + +/** + * Read the machine-id from `~/.altimate/machine-id`, minting a new random UUID + * with `flag: "wx"` (exclusive create) if the file is absent. + * + * - **Race-safe**: two concurrent callers on a fresh install converge on the + * same UUID — the winner writes, the loser re-reads. + * - **Size-capped**: reads at most 512 bytes to avoid multi-MB file attacks. + * - **UUID-validated**: rejects content that does not match RFC 4122 v4 UUID + * format (corrupt file, symlink content, etc.) and returns `""` with a warn + * log so callers can omit the field rather than propagate garbage. + * + * @param machineIdPath Override path (for tests). Defaults to `~/.altimate/machine-id`. + * @returns A v4 UUID string, or `""` if the value is invalid or unreadable. + */ +export function getOrCreateMachineId(machineIdPath?: string): string { + const idPath = machineIdPath ?? path.join(os.homedir(), ".altimate", "machine-id") + + // --- Read path --- + let raw: string | undefined + try { + // Cap read size to avoid multi-MB files (corrupt or malicious). + const stat = fs.statSync(idPath) + if (stat.size > MAX_BYTES) { + log.warn("machine-id file exceeds size limit — omitting", { path: idPath, size: stat.size }) + return "" + } + raw = fs.readFileSync(idPath, "utf8").trim() + } catch (readErr) { + const code = (readErr as NodeJS.ErrnoException)?.code + if (code !== "ENOENT") { + // EACCES, EMFILE, etc. — log and bail; we cannot create either. + log.warn("machine-id read failed", { code, path: idPath }) + return "" + } + // File absent — fall through to create path below. + raw = undefined + } + + if (raw !== undefined) { + // Validate before returning: reject corrupt or symlink-injected content. + if (!UUID_RE.test(raw)) { + log.warn("machine-id file contains non-UUID content — omitting", { path: idPath }) + return "" + } + return raw + } + + // --- Create path (ENOENT) --- + // `flag: "wx"` is atomic exclusive-create: the OS guarantees only one writer + // succeeds. The loser re-reads what the winner wrote. + const candidate = randomUUID() + fs.mkdirSync(path.dirname(idPath), { recursive: true }) + try { + fs.writeFileSync(idPath, candidate, { encoding: "utf8", flag: "wx" }) + return candidate + } catch (writeErr) { + const code = (writeErr as NodeJS.ErrnoException)?.code + if (code !== "EEXIST") { + log.warn("machine-id create failed", { code, path: idPath }) + return "" + } + // Lost the race — read what the winner wrote. + try { + const winner = fs.readFileSync(idPath, "utf8").trim() + if (!UUID_RE.test(winner)) { + log.warn("machine-id written by race winner is non-UUID — omitting", { path: idPath }) + return "" + } + return winner + } catch (rereadErr) { + log.warn("machine-id re-read after race failed", { + code: (rereadErr as NodeJS.ErrnoException)?.code, + path: idPath, + }) + return "" + } + } +} diff --git a/packages/opencode/src/cli/welcome.ts b/packages/opencode/src/cli/welcome.ts index 2b08a79b3..ea968822d 100644 --- a/packages/opencode/src/cli/welcome.ts +++ b/packages/opencode/src/cli/welcome.ts @@ -6,6 +6,8 @@ import { EOL } from "os" // altimate_change start — import Telemetry for first_launch event import { Telemetry } from "../altimate/telemetry" // altimate_change end +// altimate_change — import shared machine-id utility so the path is canonical across all call sites +import { getOrCreateMachineId } from "../altimate/util/machine-id" const APP_NAME = "altimate-code" const MARKER_FILE = ".installed-version" @@ -39,12 +41,17 @@ export function showWelcomeBannerIfNeeded(): void { // Remove marker first to avoid showing twice even if display fails fs.unlinkSync(markerPath) - // altimate_change start — use ~/.altimate/machine-id existence as a proxy for upgrade vs fresh install - // Since postinstall.mjs always writes the current version to the marker file, we can't reliably - // use installedVersion !== currentVersion for release builds. Instead, if machine-id exists, - // they've run the CLI before. - const machineIdPath = path.join(os.homedir(), ".altimate", "machine-id") - const isUpgrade = fs.existsSync(machineIdPath) + // altimate_change start — use getOrCreateMachineId() as the upgrade probe so the path is + // canonical and consistent with telemetry. Returns "" on a fresh install (before the file + // exists), in which case we treat this as a new install. On any subsequent run the file exists + // (minted by telemetry on first run) so getOrCreateMachineId() returns the existing UUID, + // indicating an upgrade. + // NOTE: welcome.ts runs before telemetry.doInit(). Calling getOrCreateMachineId() here mints + // the machine-id on fresh installs so telemetry has the ID ready when doInit() runs. + const machineId = process.env.ALTIMATE_TELEMETRY_DISABLED !== "true" + ? getOrCreateMachineId() + : fs.existsSync(path.join(os.homedir(), ".altimate", "machine-id")) ? "exists" : "" + const isUpgrade = machineId !== "" // altimate_change end // altimate_change start — track first launch for new user counting (privacy-safe: only version + machine_id) diff --git a/packages/opencode/test/altimate/altimate-plugin.test.ts b/packages/opencode/test/altimate/altimate-plugin.test.ts index 06e46f4f2..b9a471c0f 100644 --- a/packages/opencode/test/altimate/altimate-plugin.test.ts +++ b/packages/opencode/test/altimate/altimate-plugin.test.ts @@ -9,7 +9,7 @@ describe("buildCliContext", () => { test("returns a valid base64url-encoded JSON blob with machine_id", () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-")) const idPath = path.join(tmpDir, "machine-id") - fs.writeFileSync(idPath, "test-uuid-1234", "utf8") + fs.writeFileSync(idPath, "550e8400-e29b-41d4-a716-446655440000", "utf8") const encoded = buildCliContext(idPath) @@ -18,7 +18,7 @@ describe("buildCliContext", () => { const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record expect(ctx["v"]).toBe(1) - expect(ctx["machine_id"]).toBe("test-uuid-1234") + expect(ctx["machine_id"]).toBe("550e8400-e29b-41d4-a716-446655440000") expect(typeof ctx["cli_version"]).toBe("string") fs.rmSync(tmpDir, { recursive: true, force: true }) @@ -47,12 +47,12 @@ describe("buildCliContext", () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-ws-")) const idPath = path.join(tmpDir, "machine-id") // Many editors/tools write a trailing newline - fs.writeFileSync(idPath, " trimmed-uuid \n", "utf8") + fs.writeFileSync(idPath, " 550e8400-e29b-41d4-a716-446655440001 \n", "utf8") const encoded = buildCliContext(idPath) const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record - expect(ctx["machine_id"]).toBe("trimmed-uuid") + expect(ctx["machine_id"]).toBe("550e8400-e29b-41d4-a716-446655440001") fs.rmSync(tmpDir, { recursive: true, force: true }) }) @@ -74,7 +74,7 @@ describe("buildCliContext", () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-disabled-")) const idPath = path.join(tmpDir, "machine-id") - fs.writeFileSync(idPath, "should-not-appear", "utf8") + fs.writeFileSync(idPath, "550e8400-e29b-41d4-a716-446655440004", "utf8") const encoded = buildCliContext(idPath) const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record @@ -91,12 +91,12 @@ describe("buildCliContext", () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-enabled-")) const idPath = path.join(tmpDir, "machine-id") - fs.writeFileSync(idPath, "expected-uuid", "utf8") + fs.writeFileSync(idPath, "550e8400-e29b-41d4-a716-446655440002", "utf8") const encoded = buildCliContext(idPath) const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record - expect(ctx["machine_id"]).toBe("expected-uuid") + expect(ctx["machine_id"]).toBe("550e8400-e29b-41d4-a716-446655440002") fs.rmSync(tmpDir, { recursive: true, force: true }) }) @@ -107,9 +107,9 @@ describe("getOrCreateMachineId", () => { test("returns existing id when file is present", () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-")) const idPath = path.join(tmpDir, "machine-id") - fs.writeFileSync(idPath, "existing-uuid\n", "utf8") + fs.writeFileSync(idPath, "550e8400-e29b-41d4-a716-446655440003\n", "utf8") - expect(getOrCreateMachineId(idPath)).toBe("existing-uuid") + expect(getOrCreateMachineId(idPath)).toBe("550e8400-e29b-41d4-a716-446655440003") fs.rmSync(tmpDir, { recursive: true, force: true }) }) @@ -148,7 +148,7 @@ describe("buildAuthorizeUrl", () => { test("URL contains cli_context param that decodes to valid JSON", () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-auth-url-")) const idPath = path.join(tmpDir, "machine-id") - fs.writeFileSync(idPath, "url-test-uuid", "utf8") + fs.writeFileSync(idPath, "550e8400-e29b-41d4-a716-446655440005", "utf8") // Temporarily override machine-id path via a patched buildCliContext call // by providing a custom machine_id file — buildAuthorizeUrl uses buildCliContext() @@ -186,3 +186,62 @@ describe("buildAuthorizeUrl", () => { expect(parsed.searchParams.has("cli_context")).toBe(true) }) }) + +// altimate_change — additional failure mode tests for getOrCreateMachineId (MINOR 8) +describe("getOrCreateMachineId — failure modes", () => { + test("returns empty string for non-UUID content without throwing", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-invalid-")) + const idPath = path.join(tmpDir, "machine-id") + // Write garbage that is not a v4 UUID + fs.writeFileSync(idPath, "not-a-uuid-at-all", "utf8") + + const id = getOrCreateMachineId(idPath) + + // Must return empty string (warn logged internally, not thrown) + expect(id).toBe("") + + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + + test("returns empty string for oversized file without throwing", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-big-")) + const idPath = path.join(tmpDir, "machine-id") + // Write a file larger than the 512-byte cap + fs.writeFileSync(idPath, "x".repeat(513), "utf8") + + const id = getOrCreateMachineId(idPath) + + // Must return empty string — oversized content rejected + expect(id).toBe("") + + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + + test("returns empty string when file has valid UUID format but wrong version", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-v1-")) + const idPath = path.join(tmpDir, "machine-id") + // v1 UUID (time-based, not version 4) — third group starts with 1, not 4 + fs.writeFileSync(idPath, "550e8400-e29b-11d4-a716-446655440000", "utf8") + + const id = getOrCreateMachineId(idPath) + + // Strict UUID v4 validation rejects this + expect(id).toBe("") + + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + + test("buildCliContext omits machine_id when file contains non-UUID content", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-corrupt-")) + const idPath = path.join(tmpDir, "machine-id") + fs.writeFileSync(idPath, "not-a-uuid", "utf8") + + const encoded = buildCliContext(idPath) + const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record + + // machine_id must be absent — non-UUID content is rejected + expect(Object.prototype.hasOwnProperty.call(ctx, "machine_id")).toBe(false) + + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) +}) From 43fb3436c6ca1285ee3c2419769e511822106e5a Mon Sep 17 00:00:00 2001 From: Sarav Date: Wed, 5 Aug 2026 11:37:10 +0530 Subject: [PATCH 4/6] fix: [AI] address second-round review on cli_context auth URL param - honour config.telemetry.disabled (not just the env var) in buildCliContext by awaiting Config.get(), mirroring telemetry/index.ts::doInit - move cli_context into the URL fragment (#cli_context=) so the durable machine_id never reaches server access logs or the Referer header - reject symlinks / non-regular files via lstat in getOrCreateMachineId - fix welcome.ts fresh-install probe: use existsSync before minting so new users are no longer misclassified as upgrades - add failure-mode tests (empty file, directory, symlink); update tests for async buildCliContext/buildAuthorizeUrl and the fragment-based URL Co-Authored-By: Claude Opus 4.8 --- .../opencode/src/altimate/plugin/altimate.ts | 53 +++++---- .../opencode/src/altimate/util/machine-id.ts | 14 ++- packages/opencode/src/cli/welcome.ts | 19 ++-- .../test/altimate/altimate-plugin.test.ts | 103 +++++++++++++----- 4 files changed, 129 insertions(+), 60 deletions(-) diff --git a/packages/opencode/src/altimate/plugin/altimate.ts b/packages/opencode/src/altimate/plugin/altimate.ts index cde3dce62..0cd41262c 100644 --- a/packages/opencode/src/altimate/plugin/altimate.ts +++ b/packages/opencode/src/altimate/plugin/altimate.ts @@ -7,6 +7,7 @@ import { AltimateApi } from "../api/client" import * as OnboardingTelemetry from "../telemetry/onboarding" // altimate_change — shared machine-id helper (race-safe, UUID-validated, size-capped) import { getOrCreateMachineId } from "../util/machine-id" +import { Config } from "@/config/config" import { InstallationVersion } from "@opencode-ai/core/installation/version" import { Log } from "@/altimate/util/log" @@ -63,24 +64,31 @@ export { getOrCreateMachineId } from "../util/machine-id" // After sign-in, the frontend calls posthog.alias(email, machine_id) to associate // the device with the authenticated account in product analytics. // -// Privacy note: cli_context is sent as a URL query parameter to /register. -// The machine_id is a crypto.randomUUID() — non-PII by construction. We keep it -// in the query string (rather than a fragment, which JS can read but servers -// cannot log) because the /register route must have Referrer-Policy: no-referrer -// on all outbound links and telemetry is opt-out, not opt-in. If those server-side -// controls are ever removed, move this to a URL fragment (#cli_context=...) and -// update the frontend to read window.location.hash instead of searchParams. -export function buildCliContext(machineIdPath?: string): string { - // altimate_change start — honour both telemetry opt-out gates: - // 1. ALTIMATE_TELEMETRY_DISABLED=true env var (matches telemetry/index.ts::doInit) - // 2. Config-based disabled flag (checked by telemetry/index.ts via Config.get()) - // We intentionally only gate on the env var here because Config.get() is async - // and buildCliContext is called synchronously during URL construction. The env - // var is the documented, widely-supported escape hatch for scripts and CI. - // Config-based opt-out users who also want machine_id suppressed in the auth - // URL should additionally set ALTIMATE_TELEMETRY_DISABLED=true. +// Privacy note: cli_context is sent in the URL *fragment* (#cli_context=...), +// not the query string. The browser never transmits a fragment to the server, +// so the machine_id — though a non-PII crypto.randomUUID() — stays out of +// app.myaltimate.com's access logs, any fronting CDN/WAF, and the Referer +// header, while remaining readable by the /register page via location.hash. +// The frontend reads it from the fragment (see useCliContext.ts). The fragment +// must be the last URL segment, after all query params. +export async function buildCliContext(machineIdPath?: string): Promise { + // altimate_change start — honour both telemetry opt-out gates, mirroring + // telemetry/index.ts::doInit exactly: + // 1. ALTIMATE_TELEMETRY_DISABLED=true env var (early, always-works escape hatch) + // 2. config.telemetry.disabled (resolved via the async Config.get()) + // Config.get() may throw outside an Instance context; treat a config failure as + // "not disabled" (same as doInit) — the env var above is the hard opt-out. + let disabled = process.env.ALTIMATE_TELEMETRY_DISABLED === "true" + if (!disabled) { + try { + const userConfig = (await Config.get()) as any + disabled = Boolean(userConfig.telemetry?.disabled) + } catch { + // Config unavailable — proceed with telemetry enabled. + } + } let machineId = "" - if (process.env.ALTIMATE_TELEMETRY_DISABLED !== "true") { + if (!disabled) { // getOrCreateMachineId returns "" on all error conditions (ENOENT excluded — // it mints a new UUID instead) and logs appropriately; no try/catch needed. machineId = getOrCreateMachineId(machineIdPath) @@ -97,12 +105,14 @@ export function buildCliContext(machineIdPath?: string): string { // altimate_change start — exported so tests can assert on the full URL shape // without duplicating the construction logic. -export function buildAuthorizeUrl(webUrl: string, redirect: string, state: string): string { +export async function buildAuthorizeUrl(webUrl: string, redirect: string, state: string): Promise { return ( `${webUrl}/register?client=altimate-code` + `&redirect=${encodeURIComponent(redirect)}` + `&state=${state}` + - `&cli_context=${encodeURIComponent(buildCliContext())}` + // Fragment (#), not a query param — keeps the durable machine_id out of + // server access logs / Referer. Must stay last, after all query params. + `#cli_context=${encodeURIComponent(await buildCliContext())}` ) } // altimate_change end @@ -401,7 +411,7 @@ export async function AltimateAuthPlugin(_input: PluginInput): Promise { const redirect = `http://127.0.0.1:${boundPort}/callback` // Land on the sign-up page and let the user choose how to authenticate // (Google today, more providers later) rather than forcing Google. - const authorizeUrl = buildAuthorizeUrl(webUrl, redirect, state) + const authorizeUrl = await buildAuthorizeUrl(webUrl, redirect, state) // Try to open the browser. Failure is silent because the URL is // already surfaced elsewhere: the auth dialog in packages/tui/src/ @@ -420,7 +430,8 @@ export async function AltimateAuthPlugin(_input: PluginInput): Promise { // attempted". open() failures are swallowed above (the URL is also printed for the // user to paste), so this fires even when no browser actually launched. // The URL is never sent — it carries the CSRF `state`. - if (OnboardingTelemetry.isFunnelActive()) void OnboardingTelemetry.emit({ type: "gateway_device_code_issued" }) + if (OnboardingTelemetry.isFunnelActive()) + void OnboardingTelemetry.emit({ type: "gateway_device_code_issued" }) // One outcome per attempt. callback() closes over `result` and re-runs its whole body // on every invocation, so a repeated call would otherwise re-emit completion/failure diff --git a/packages/opencode/src/altimate/util/machine-id.ts b/packages/opencode/src/altimate/util/machine-id.ts index 4d503ff50..8694886fd 100644 --- a/packages/opencode/src/altimate/util/machine-id.ts +++ b/packages/opencode/src/altimate/util/machine-id.ts @@ -23,9 +23,11 @@ const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f * * - **Race-safe**: two concurrent callers on a fresh install converge on the * same UUID — the winner writes, the loser re-reads. + * - **Regular-file-only**: uses `lstat` and rejects symlinks / non-regular + * files rather than following them to an attacker-chosen target. * - **Size-capped**: reads at most 512 bytes to avoid multi-MB file attacks. * - **UUID-validated**: rejects content that does not match RFC 4122 v4 UUID - * format (corrupt file, symlink content, etc.) and returns `""` with a warn + * format (corrupt file, injected content, etc.) and returns `""` with a warn * log so callers can omit the field rather than propagate garbage. * * @param machineIdPath Override path (for tests). Defaults to `~/.altimate/machine-id`. @@ -37,8 +39,14 @@ export function getOrCreateMachineId(machineIdPath?: string): string { // --- Read path --- let raw: string | undefined try { - // Cap read size to avoid multi-MB files (corrupt or malicious). - const stat = fs.statSync(idPath) + // lstat (not stat) so a symlink is inspected as itself rather than followed + // to an attacker-chosen target. Reject anything that is not a regular file + // (symlink, directory, socket, …) and cap read size to avoid multi-MB files. + const stat = fs.lstatSync(idPath) + if (!stat.isFile()) { + log.warn("machine-id is not a regular file — omitting", { path: idPath }) + return "" + } if (stat.size > MAX_BYTES) { log.warn("machine-id file exceeds size limit — omitting", { path: idPath, size: stat.size }) return "" diff --git a/packages/opencode/src/cli/welcome.ts b/packages/opencode/src/cli/welcome.ts index ea968822d..9cc176375 100644 --- a/packages/opencode/src/cli/welcome.ts +++ b/packages/opencode/src/cli/welcome.ts @@ -41,17 +41,14 @@ export function showWelcomeBannerIfNeeded(): void { // Remove marker first to avoid showing twice even if display fails fs.unlinkSync(markerPath) - // altimate_change start — use getOrCreateMachineId() as the upgrade probe so the path is - // canonical and consistent with telemetry. Returns "" on a fresh install (before the file - // exists), in which case we treat this as a new install. On any subsequent run the file exists - // (minted by telemetry on first run) so getOrCreateMachineId() returns the existing UUID, - // indicating an upgrade. - // NOTE: welcome.ts runs before telemetry.doInit(). Calling getOrCreateMachineId() here mints - // the machine-id on fresh installs so telemetry has the ID ready when doInit() runs. - const machineId = process.env.ALTIMATE_TELEMETRY_DISABLED !== "true" - ? getOrCreateMachineId() - : fs.existsSync(path.join(os.homedir(), ".altimate", "machine-id")) ? "exists" : "" - const isUpgrade = machineId !== "" + // altimate_change start — "upgrade" means the machine-id file already existed before this + // launch. Probe existence with existsSync FIRST — do NOT use getOrCreateMachineId() as the + // probe, because it mints the file on a fresh install and would then report every new user + // as an upgrade. After probing, mint the id (unless telemetry is opted out via env) so + // telemetry.doInit() finds it ready — welcome.ts runs before doInit(). + const machineIdPath = path.join(os.homedir(), ".altimate", "machine-id") + const isUpgrade = fs.existsSync(machineIdPath) + if (process.env.ALTIMATE_TELEMETRY_DISABLED !== "true") getOrCreateMachineId() // altimate_change end // altimate_change start — track first launch for new user counting (privacy-safe: only version + machine_id) diff --git a/packages/opencode/test/altimate/altimate-plugin.test.ts b/packages/opencode/test/altimate/altimate-plugin.test.ts index b9a471c0f..91c0916b4 100644 --- a/packages/opencode/test/altimate/altimate-plugin.test.ts +++ b/packages/opencode/test/altimate/altimate-plugin.test.ts @@ -6,12 +6,12 @@ import * as path from "path" import { buildAuthorizeUrl, buildCliContext, getOrCreateMachineId } from "../../src/altimate/plugin/altimate" describe("buildCliContext", () => { - test("returns a valid base64url-encoded JSON blob with machine_id", () => { + test("returns a valid base64url-encoded JSON blob with machine_id", async () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-")) const idPath = path.join(tmpDir, "machine-id") fs.writeFileSync(idPath, "550e8400-e29b-41d4-a716-446655440000", "utf8") - const encoded = buildCliContext(idPath) + const encoded = await buildCliContext(idPath) // base64url: only A-Z a-z 0-9 - _ (no +/=) expect(encoded).toMatch(/^[A-Za-z0-9\-_]+$/) @@ -24,13 +24,13 @@ describe("buildCliContext", () => { fs.rmSync(tmpDir, { recursive: true, force: true }) }) - test("creates machine_id file when absent and includes it in context", () => { + test("creates machine_id file when absent and includes it in context", async () => { // getOrCreateMachineId mints a UUID when the file is missing, so machine_id // is always present (as a non-empty string) unless telemetry is disabled. const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-new-")) const nonExistentPath = path.join(tmpDir, "subdir", "machine-id") - const encoded = buildCliContext(nonExistentPath) + const encoded = await buildCliContext(nonExistentPath) const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record expect(ctx["v"]).toBe(1) @@ -38,18 +38,18 @@ describe("buildCliContext", () => { expect(typeof ctx["machine_id"]).toBe("string") expect((ctx["machine_id"] as string).length).toBeGreaterThan(0) // The same id must have been written to disk for telemetry to use - expect(fs.readFileSync(nonExistentPath, "utf8").trim()).toBe(ctx["machine_id"]) + expect(fs.readFileSync(nonExistentPath, "utf8").trim()).toBe(ctx["machine_id"] as string) fs.rmSync(tmpDir, { recursive: true, force: true }) }) - test("trims whitespace from machine-id file", () => { + test("trims whitespace from machine-id file", async () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-ws-")) const idPath = path.join(tmpDir, "machine-id") // Many editors/tools write a trailing newline fs.writeFileSync(idPath, " 550e8400-e29b-41d4-a716-446655440001 \n", "utf8") - const encoded = buildCliContext(idPath) + const encoded = await buildCliContext(idPath) const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record expect(ctx["machine_id"]).toBe("550e8400-e29b-41d4-a716-446655440001") @@ -69,14 +69,14 @@ describe("buildCliContext", () => { else process.env.ALTIMATE_TELEMETRY_DISABLED = savedEnv }) - test("omits machine_id key when ALTIMATE_TELEMETRY_DISABLED=true", () => { + test("omits machine_id key when ALTIMATE_TELEMETRY_DISABLED=true", async () => { process.env.ALTIMATE_TELEMETRY_DISABLED = "true" const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-disabled-")) const idPath = path.join(tmpDir, "machine-id") fs.writeFileSync(idPath, "550e8400-e29b-41d4-a716-446655440004", "utf8") - const encoded = buildCliContext(idPath) + const encoded = await buildCliContext(idPath) const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record // machine_id must be absent when telemetry is disabled @@ -86,14 +86,14 @@ describe("buildCliContext", () => { fs.rmSync(tmpDir, { recursive: true, force: true }) }) - test("includes machine_id when ALTIMATE_TELEMETRY_DISABLED is not set", () => { + test("includes machine_id when ALTIMATE_TELEMETRY_DISABLED is not set", async () => { delete process.env.ALTIMATE_TELEMETRY_DISABLED const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-enabled-")) const idPath = path.join(tmpDir, "machine-id") fs.writeFileSync(idPath, "550e8400-e29b-41d4-a716-446655440002", "utf8") - const encoded = buildCliContext(idPath) + const encoded = await buildCliContext(idPath) const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record expect(ctx["machine_id"]).toBe("550e8400-e29b-41d4-a716-446655440002") @@ -145,7 +145,7 @@ describe("getOrCreateMachineId", () => { }) describe("buildAuthorizeUrl", () => { - test("URL contains cli_context param that decodes to valid JSON", () => { + test("URL contains cli_context param that decodes to valid JSON", async () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-auth-url-")) const idPath = path.join(tmpDir, "machine-id") fs.writeFileSync(idPath, "550e8400-e29b-41d4-a716-446655440005", "utf8") @@ -153,17 +153,24 @@ describe("buildAuthorizeUrl", () => { // Temporarily override machine-id path via a patched buildCliContext call // by providing a custom machine_id file — buildAuthorizeUrl uses buildCliContext() // internally with no path arg, so test the URL shape via the exported helper. - const url = buildAuthorizeUrl("https://app.myaltimate.com", "http://127.0.0.1:7317/callback", "test-state-abc") - - // Must contain cli_context query param - expect(url).toContain("cli_context=") + const url = await buildAuthorizeUrl( + "https://app.myaltimate.com", + "http://127.0.0.1:7317/callback", + "test-state-abc", + ) + + // cli_context rides in the fragment (#), not the query string + expect(url).toContain("#cli_context=") expect(url).toContain("client=altimate-code") expect(url).toContain("state=test-state-abc") expect(url).toContain("redirect=") - // Extract and decode cli_context + // The durable id must NOT be in the query string (it would hit access logs) const parsed = new URL(url) - const encoded = parsed.searchParams.get("cli_context") + expect(parsed.searchParams.has("cli_context")).toBe(false) + + // Extract and decode cli_context from the fragment + const encoded = new URLSearchParams(parsed.hash.replace(/^#/, "")).get("cli_context") expect(encoded).toBeTruthy() const ctx = JSON.parse(Buffer.from(encoded!, "base64url").toString("utf8")) as Record @@ -178,12 +185,14 @@ describe("buildAuthorizeUrl", () => { fs.rmSync(tmpDir, { recursive: true, force: true }) }) - test("deleting cli_context line would cause cli_context param to be absent — URL integration is guarded", () => { - // This test asserts that buildAuthorizeUrl really does embed cli_context. - // If the &cli_context=... line were removed from buildAuthorizeUrl, this test fails. - const url = buildAuthorizeUrl("https://app.myaltimate.com", "http://127.0.0.1:7317/callback", "state-xyz") + test("deleting cli_context line would cause cli_context to be absent — URL integration is guarded", async () => { + // This test asserts that buildAuthorizeUrl really does embed cli_context in + // the fragment. If the #cli_context=... line were removed, this test fails. + const url = await buildAuthorizeUrl("https://app.myaltimate.com", "http://127.0.0.1:7317/callback", "state-xyz") const parsed = new URL(url) - expect(parsed.searchParams.has("cli_context")).toBe(true) + expect(new URLSearchParams(parsed.hash.replace(/^#/, "")).has("cli_context")).toBe(true) + // And never in the query string, where it would be logged. + expect(parsed.searchParams.has("cli_context")).toBe(false) }) }) @@ -231,12 +240,56 @@ describe("getOrCreateMachineId — failure modes", () => { fs.rmSync(tmpDir, { recursive: true, force: true }) }) - test("buildCliContext omits machine_id when file contains non-UUID content", () => { + test("returns empty string for an empty file without minting over it", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-empty-")) + const idPath = path.join(tmpDir, "machine-id") + // 0-byte file exists — must NOT be treated as absent (no exclusive-create mint) + fs.writeFileSync(idPath, "", "utf8") + + const id = getOrCreateMachineId(idPath) + + // Empty content fails UUID validation → "" (and the file is left untouched) + expect(id).toBe("") + expect(fs.readFileSync(idPath, "utf8")).toBe("") + + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + + test("returns empty string when the path is a directory", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-dir-")) + const idPath = path.join(tmpDir, "machine-id") + // A directory at the machine-id path — lstat rejects it as a non-regular file + fs.mkdirSync(idPath) + + const id = getOrCreateMachineId(idPath) + + expect(id).toBe("") + + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + + test("returns empty string when the path is a symlink (not followed)", () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-symlink-")) + // Target holds a perfectly valid UUID — lstat must still reject the symlink + // itself rather than following it to read the target. + const targetPath = path.join(tmpDir, "target") + fs.writeFileSync(targetPath, "550e8400-e29b-41d4-a716-446655440099", "utf8") + const linkPath = path.join(tmpDir, "machine-id") + fs.symlinkSync(targetPath, linkPath) + + const id = getOrCreateMachineId(linkPath) + + expect(id).toBe("") + + fs.rmSync(tmpDir, { recursive: true, force: true }) + }) + + test("buildCliContext omits machine_id when file contains non-UUID content", async () => { const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-corrupt-")) const idPath = path.join(tmpDir, "machine-id") fs.writeFileSync(idPath, "not-a-uuid", "utf8") - const encoded = buildCliContext(idPath) + const encoded = await buildCliContext(idPath) const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record // machine_id must be absent — non-UUID content is rejected From ffe8b1faafb93da2830e5f51527cddeb5eb54b6a Mon Sep 17 00:00:00 2001 From: Sarav Date: Fri, 7 Aug 2026 07:51:29 +0530 Subject: [PATCH 5/6] fix: [AI] address review round 3 on cli_context auth - machine-id: move mkdirSync inside the try/catch so a read-only $HOME / restricted container returns "" instead of throwing (was breaking sign-in via buildCliContext -> buildAuthorizeUrl -> authorize) - buildCliContext: fail CLOSED when Config.get() throws (the plugin can run in the server worker where it does) so a config-opted-out user's id is never sent - welcome.ts: stop minting the machine-id; delegate creation to Telemetry.doInit (which resolves env + config); keep existsSync as the upgrade probe - buildAuthorizeUrl: accept an optional machineIdPath forwarded to buildCliContext; encode the state param - docs: name both opt-out mechanisms (env var AND telemetry.disabled config) and reconcile the PostHog vs App Insights destinations - tests: use the repo tmpdir() fixture (no $HOME writes), real wx/EEXIST race and mkdir-EACCES branches via spyOn, config-opt-out + fail-closed cases, non-vacuous assertions, and guard against a developer's exported ALTIMATE_TELEMETRY_DISABLED - remove the dead getOrCreateMachineId re-export; import from util/machine-id Co-Authored-By: Claude Opus 4.8 --- docs/docs/reference/security-faq.md | 4 +- docs/docs/reference/telemetry.md | 2 +- .../opencode/src/altimate/plugin/altimate.ts | 53 ++- .../opencode/src/altimate/util/machine-id.ts | 7 +- packages/opencode/src/cli/welcome.ts | 12 +- .../test/altimate/altimate-plugin.test.ts | 360 ++++++++++-------- 6 files changed, 244 insertions(+), 194 deletions(-) diff --git a/docs/docs/reference/security-faq.md b/docs/docs/reference/security-faq.md index b81ced97f..5825730d5 100644 --- a/docs/docs/reference/security-faq.md +++ b/docs/docs/reference/security-faq.md @@ -130,9 +130,9 @@ export ALTIMATE_TELEMETRY_DISABLED=true - **Logged-in users:** Your email is SHA-256 hashed before sending. We never see your raw email. - **Anonymous users:** A random UUID (`crypto.randomUUID()`) is generated on first run and stored at `~/.altimate/machine-id`. This is NOT tied to your hardware, OS, or identity — it's purely random. -- **Both identifiers** are only sent when telemetry is enabled. Disable with `ALTIMATE_TELEMETRY_DISABLED=true`. +- **Both identifiers** are only sent when telemetry is enabled. Disable via `ALTIMATE_TELEMETRY_DISABLED=true` or the `telemetry.disabled` config option. - **No fingerprinting:** We do not use browser fingerprinting, hardware IDs, MAC addresses, or IP-based tracking. -- **CLI auth flow:** When you sign in via `altimate auth login`, the anonymous machine ID is included in the authorization URL and associated with your account in product analytics for funnel analysis. This is suppressed when `ALTIMATE_TELEMETRY_DISABLED=true` is set — the machine ID is omitted from the URL entirely. +- **CLI auth flow:** When you sign in via `altimate auth login`, the anonymous machine ID is included in the authorization URL and associated with your account in product analytics for funnel analysis. This is suppressed when you disable telemetry — via `ALTIMATE_TELEMETRY_DISABLED=true` or the `telemetry.disabled` config option — and the machine ID is omitted from the URL entirely. ### What happens on first launch? diff --git a/docs/docs/reference/telemetry.md b/docs/docs/reference/telemetry.md index 110d739fb..d7c52706b 100644 --- a/docs/docs/reference/telemetry.md +++ b/docs/docs/reference/telemetry.md @@ -151,7 +151,7 @@ Both identifiers are only sent when telemetry is enabled. Disable telemetry enti ### CLI Authentication Flow -When you sign in using the CLI browser auth flow (`altimate auth login`), an anonymized session identifier (the `machine-id` UUID) is included in the authorization URL and associated with your account in product analytics. This is used solely to correlate CLI install events with authenticated accounts in aggregate funnel analytics — it is never used for tracking, advertising, or cross-site identification. Respecting `ALTIMATE_TELEMETRY_DISABLED=true` suppresses this: when telemetry opt-out is set, the machine ID is omitted from the authorization URL entirely. +When you sign in using the CLI browser auth flow (`altimate auth login`), an anonymized session identifier (the `machine-id` UUID) is included in the authorization URL and associated with your account in product analytics. This is used solely to correlate CLI install events with authenticated accounts in aggregate funnel analytics — it is never used for tracking, advertising, or cross-site identification. Your telemetry opt-out suppresses this: when you disable telemetry — via `ALTIMATE_TELEMETRY_DISABLED=true` **or** the `telemetry.disabled` config option — the machine ID is omitted from the authorization URL entirely. The machine ID is associated with your account in PostHog for this funnel analysis, separate from the Azure Application Insights pipeline used for other CLI telemetry events. ### Data Retention diff --git a/packages/opencode/src/altimate/plugin/altimate.ts b/packages/opencode/src/altimate/plugin/altimate.ts index 0cd41262c..059b89e76 100644 --- a/packages/opencode/src/altimate/plugin/altimate.ts +++ b/packages/opencode/src/altimate/plugin/altimate.ts @@ -54,37 +54,43 @@ const DEFAULT_API_URL = "https://api.myaltimate.com" const log = Log.create({ service: "altimate-plugin" }) -// altimate_change — getOrCreateMachineId is now in util/machine-id.ts (re-exported -// from there so existing test imports that reference this module continue to work). -export { getOrCreateMachineId } from "../util/machine-id" - // Builds a base64url-encoded context blob for correlating this browser auth // session with CLI telemetry in PostHog. The machine_id is a random UUID // stored at ~/.altimate/machine-id — not tied to hardware, OS, or user identity. -// After sign-in, the frontend calls posthog.alias(email, machine_id) to associate -// the device with the authenticated account in product analytics. +// After sign-in the frontend registers it as the `cli_machine_id` PostHog +// super-property so the CLI device is attributed to the authenticated account +// in aggregate funnel analytics. // // Privacy note: cli_context is sent in the URL *fragment* (#cli_context=...), // not the query string. The browser never transmits a fragment to the server, // so the machine_id — though a non-PII crypto.randomUUID() — stays out of // app.myaltimate.com's access logs, any fronting CDN/WAF, and the Referer // header, while remaining readable by the /register page via location.hash. -// The frontend reads it from the fragment (see useCliContext.ts). The fragment -// must be the last URL segment, after all query params. +// +// Frontend decode contract (implemented in monorepo useCliContext.ts / +// cliContext.ts): the value is base64url (not standard base64); the consumer +// must catch decode/JSON errors, require `v === 1`, validate that `machine_id` +// and `cli_version` are strings, treat `cli_version: "local"` as a valid dev +// build, and treat the payload as untrusted (anyone can craft a URL). An absent +// `machine_id` means "do not attribute" — it must never be aliased on. export async function buildCliContext(machineIdPath?: string): Promise { // altimate_change start — honour both telemetry opt-out gates, mirroring - // telemetry/index.ts::doInit exactly: - // 1. ALTIMATE_TELEMETRY_DISABLED=true env var (early, always-works escape hatch) + // telemetry/index.ts::doInit: + // 1. ALTIMATE_TELEMETRY_DISABLED=true env var (always-works hard opt-out) // 2. config.telemetry.disabled (resolved via the async Config.get()) - // Config.get() may throw outside an Instance context; treat a config failure as - // "not disabled" (same as doInit) — the env var above is the hard opt-out. let disabled = process.env.ALTIMATE_TELEMETRY_DISABLED === "true" if (!disabled) { try { const userConfig = (await Config.get()) as any disabled = Boolean(userConfig.telemetry?.disabled) } catch { - // Config unavailable — proceed with telemetry enabled. + // Config unreadable here — this plugin can run in the server worker where + // Config.get() throws "InstanceRef not provided". Fail CLOSED: omit the + // durable machine_id. A missed correlation is preferable to transmitting a + // stable cross-session device identifier for a user who may have opted out + // via config. (Intentionally stricter than doInit's fail-open, which + // governs single events rather than a persistent identifier.) + disabled = true } } let machineId = "" @@ -94,9 +100,9 @@ export async function buildCliContext(machineIdPath?: string): Promise { machineId = getOrCreateMachineId(machineIdPath) } // altimate_change end - // altimate_change start — omit machine_id key when empty (matches telemetry - // module pattern: `...(machineId && { machine_id: machineId })`). Sending "" - // is meaningless for posthog.alias() and misleads downstream consumers. + // altimate_change start — omit machine_id when empty (matches the telemetry + // module's `...(machineId && { machine_id })`). An empty value is meaningless + // to the frontend super-property registration and must not be sent. const ctx: Record = { v: 1, cli_version: InstallationVersion } if (machineId) ctx.machine_id = machineId // altimate_change end @@ -104,15 +110,22 @@ export async function buildCliContext(machineIdPath?: string): Promise { } // altimate_change start — exported so tests can assert on the full URL shape -// without duplicating the construction logic. -export async function buildAuthorizeUrl(webUrl: string, redirect: string, state: string): Promise { +// without duplicating the construction logic. `machineIdPath` is forwarded to +// buildCliContext so tests can point at a temp file instead of writing a real +// id into the runner's $HOME. +export async function buildAuthorizeUrl( + webUrl: string, + redirect: string, + state: string, + machineIdPath?: string, +): Promise { return ( `${webUrl}/register?client=altimate-code` + `&redirect=${encodeURIComponent(redirect)}` + - `&state=${state}` + + `&state=${encodeURIComponent(state)}` + // Fragment (#), not a query param — keeps the durable machine_id out of // server access logs / Referer. Must stay last, after all query params. - `#cli_context=${encodeURIComponent(await buildCliContext())}` + `#cli_context=${encodeURIComponent(await buildCliContext(machineIdPath))}` ) } // altimate_change end diff --git a/packages/opencode/src/altimate/util/machine-id.ts b/packages/opencode/src/altimate/util/machine-id.ts index 8694886fd..5b6ff1d39 100644 --- a/packages/opencode/src/altimate/util/machine-id.ts +++ b/packages/opencode/src/altimate/util/machine-id.ts @@ -75,9 +75,14 @@ export function getOrCreateMachineId(machineIdPath?: string): string { // --- Create path (ENOENT) --- // `flag: "wx"` is atomic exclusive-create: the OS guarantees only one writer // succeeds. The loser re-reads what the winner wrote. + // + // mkdirSync MUST be inside this try: on a read-only $HOME, a restricted + // container, or a full disk it throws (EACCES/EROFS/ENOSPC), and the module's + // contract is to return "" on every error — never propagate. The auth path + // (buildCliContext) relies on this and no longer wraps the call itself. const candidate = randomUUID() - fs.mkdirSync(path.dirname(idPath), { recursive: true }) try { + fs.mkdirSync(path.dirname(idPath), { recursive: true }) fs.writeFileSync(idPath, candidate, { encoding: "utf8", flag: "wx" }) return candidate } catch (writeErr) { diff --git a/packages/opencode/src/cli/welcome.ts b/packages/opencode/src/cli/welcome.ts index 9cc176375..84a20405d 100644 --- a/packages/opencode/src/cli/welcome.ts +++ b/packages/opencode/src/cli/welcome.ts @@ -6,8 +6,6 @@ import { EOL } from "os" // altimate_change start — import Telemetry for first_launch event import { Telemetry } from "../altimate/telemetry" // altimate_change end -// altimate_change — import shared machine-id utility so the path is canonical across all call sites -import { getOrCreateMachineId } from "../altimate/util/machine-id" const APP_NAME = "altimate-code" const MARKER_FILE = ".installed-version" @@ -42,13 +40,13 @@ export function showWelcomeBannerIfNeeded(): void { fs.unlinkSync(markerPath) // altimate_change start — "upgrade" means the machine-id file already existed before this - // launch. Probe existence with existsSync FIRST — do NOT use getOrCreateMachineId() as the - // probe, because it mints the file on a fresh install and would then report every new user - // as an upgrade. After probing, mint the id (unless telemetry is opted out via env) so - // telemetry.doInit() finds it ready — welcome.ts runs before doInit(). + // launch. Probe existence with existsSync only — do NOT mint here. Minting is owned by + // Telemetry.doInit(), which resolves the full opt-out policy (env var AND config) before + // creating the file; minting here would duplicate that decision under a weaker gate and + // create the id for a config-opted-out user. The first_launch machine_id is attached at + // flush time from telemetry module state, so it does not depend on minting here. const machineIdPath = path.join(os.homedir(), ".altimate", "machine-id") const isUpgrade = fs.existsSync(machineIdPath) - if (process.env.ALTIMATE_TELEMETRY_DISABLED !== "true") getOrCreateMachineId() // altimate_change end // altimate_change start — track first launch for new user counting (privacy-safe: only version + machine_id) diff --git a/packages/opencode/test/altimate/altimate-plugin.test.ts b/packages/opencode/test/altimate/altimate-plugin.test.ts index 91c0916b4..7bdcbde9a 100644 --- a/packages/opencode/test/altimate/altimate-plugin.test.ts +++ b/packages/opencode/test/altimate/altimate-plugin.test.ts @@ -1,15 +1,43 @@ // altimate_change — tests for cli_context auth URL parameter -import { afterEach, beforeEach, describe, expect, test } from "bun:test" -import * as fs from "fs" -import * as os from "os" -import * as path from "path" -import { buildAuthorizeUrl, buildCliContext, getOrCreateMachineId } from "../../src/altimate/plugin/altimate" +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test" +import fs from "fs" +import path from "path" +import { buildAuthorizeUrl, buildCliContext } from "../../src/altimate/plugin/altimate" +import { getOrCreateMachineId } from "../../src/altimate/util/machine-id" +import { Config } from "../../src/config/config" +import { tmpdir } from "../fixture/fixture" + +const VALID_UUID = "550e8400-e29b-41d4-a716-446655440000" + +// buildCliContext resolves the telemetry opt-out via Config.get(), which throws +// "InstanceRef not provided" in the unit-test context and — by design — fails +// CLOSED, omitting machine_id. Stub Config.get() to an enabled config so the +// machine_id path is exercised; individual opt-out tests override the stub. +function stubConfig(impl: () => Promise) { + return spyOn(Config, "get").mockImplementation(impl as never) +} describe("buildCliContext", () => { + let cfg: ReturnType + let savedDisabled: string | undefined + + beforeEach(() => { + // Clear any real opt-out from the dev's shell so these "telemetry enabled" + // tests are not silently broken by an exported ALTIMATE_TELEMETRY_DISABLED. + savedDisabled = process.env.ALTIMATE_TELEMETRY_DISABLED + delete process.env.ALTIMATE_TELEMETRY_DISABLED + cfg = stubConfig(async () => ({})) + }) + afterEach(() => { + cfg.mockRestore() + if (savedDisabled === undefined) delete process.env.ALTIMATE_TELEMETRY_DISABLED + else process.env.ALTIMATE_TELEMETRY_DISABLED = savedDisabled + }) + test("returns a valid base64url-encoded JSON blob with machine_id", async () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-")) - const idPath = path.join(tmpDir, "machine-id") - fs.writeFileSync(idPath, "550e8400-e29b-41d4-a716-446655440000", "utf8") + await using dir = await tmpdir() + const idPath = path.join(dir.path, "machine-id") + fs.writeFileSync(idPath, VALID_UUID, "utf8") const encoded = await buildCliContext(idPath) @@ -18,43 +46,33 @@ describe("buildCliContext", () => { const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record expect(ctx["v"]).toBe(1) - expect(ctx["machine_id"]).toBe("550e8400-e29b-41d4-a716-446655440000") + expect(ctx["machine_id"]).toBe(VALID_UUID) expect(typeof ctx["cli_version"]).toBe("string") - - fs.rmSync(tmpDir, { recursive: true, force: true }) }) test("creates machine_id file when absent and includes it in context", async () => { - // getOrCreateMachineId mints a UUID when the file is missing, so machine_id - // is always present (as a non-empty string) unless telemetry is disabled. - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-new-")) - const nonExistentPath = path.join(tmpDir, "subdir", "machine-id") + await using dir = await tmpdir() + const nonExistentPath = path.join(dir.path, "subdir", "machine-id") const encoded = await buildCliContext(nonExistentPath) const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record expect(ctx["v"]).toBe(1) - // machine_id must be present and non-empty (newly minted UUID) expect(typeof ctx["machine_id"]).toBe("string") expect((ctx["machine_id"] as string).length).toBeGreaterThan(0) - // The same id must have been written to disk for telemetry to use + // The same id must have been written to disk for telemetry to reuse expect(fs.readFileSync(nonExistentPath, "utf8").trim()).toBe(ctx["machine_id"] as string) - - fs.rmSync(tmpDir, { recursive: true, force: true }) }) test("trims whitespace from machine-id file", async () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-ws-")) - const idPath = path.join(tmpDir, "machine-id") - // Many editors/tools write a trailing newline - fs.writeFileSync(idPath, " 550e8400-e29b-41d4-a716-446655440001 \n", "utf8") + await using dir = await tmpdir() + const idPath = path.join(dir.path, "machine-id") + fs.writeFileSync(idPath, ` ${VALID_UUID} \n`, "utf8") const encoded = await buildCliContext(idPath) const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record - expect(ctx["machine_id"]).toBe("550e8400-e29b-41d4-a716-446655440001") - - fs.rmSync(tmpDir, { recursive: true, force: true }) + expect(ctx["machine_id"]).toBe(VALID_UUID) }) describe("telemetry opt-out", () => { @@ -63,132 +81,196 @@ describe("buildCliContext", () => { beforeEach(() => { savedEnv = process.env.ALTIMATE_TELEMETRY_DISABLED }) - afterEach(() => { if (savedEnv === undefined) delete process.env.ALTIMATE_TELEMETRY_DISABLED else process.env.ALTIMATE_TELEMETRY_DISABLED = savedEnv }) - test("omits machine_id key when ALTIMATE_TELEMETRY_DISABLED=true", async () => { + test("omits machine_id when ALTIMATE_TELEMETRY_DISABLED=true", async () => { process.env.ALTIMATE_TELEMETRY_DISABLED = "true" - - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-disabled-")) - const idPath = path.join(tmpDir, "machine-id") - fs.writeFileSync(idPath, "550e8400-e29b-41d4-a716-446655440004", "utf8") + await using dir = await tmpdir() + const idPath = path.join(dir.path, "machine-id") + fs.writeFileSync(idPath, VALID_UUID, "utf8") const encoded = await buildCliContext(idPath) const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record - // machine_id must be absent when telemetry is disabled expect(Object.prototype.hasOwnProperty.call(ctx, "machine_id")).toBe(false) expect(ctx["v"]).toBe(1) - - fs.rmSync(tmpDir, { recursive: true, force: true }) }) - test("includes machine_id when ALTIMATE_TELEMETRY_DISABLED is not set", async () => { + test("omits machine_id when config.telemetry.disabled is set", async () => { delete process.env.ALTIMATE_TELEMETRY_DISABLED + cfg.mockImplementation((async () => ({ telemetry: { disabled: true } })) as never) + await using dir = await tmpdir() + const idPath = path.join(dir.path, "machine-id") + fs.writeFileSync(idPath, VALID_UUID, "utf8") - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-enabled-")) - const idPath = path.join(tmpDir, "machine-id") - fs.writeFileSync(idPath, "550e8400-e29b-41d4-a716-446655440002", "utf8") + const encoded = await buildCliContext(idPath) + const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record + + expect(Object.prototype.hasOwnProperty.call(ctx, "machine_id")).toBe(false) + }) + + test("omits machine_id when Config.get() throws (fails CLOSED in the worker)", async () => { + delete process.env.ALTIMATE_TELEMETRY_DISABLED + cfg.mockImplementation((async () => { + throw new Error("InstanceRef not provided") + }) as never) + await using dir = await tmpdir() + const idPath = path.join(dir.path, "machine-id") + fs.writeFileSync(idPath, VALID_UUID, "utf8") const encoded = await buildCliContext(idPath) const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record - expect(ctx["machine_id"]).toBe("550e8400-e29b-41d4-a716-446655440002") + // Config unreadable → must NOT transmit the durable id. + expect(Object.prototype.hasOwnProperty.call(ctx, "machine_id")).toBe(false) + }) - fs.rmSync(tmpDir, { recursive: true, force: true }) + test("includes machine_id when telemetry is enabled", async () => { + delete process.env.ALTIMATE_TELEMETRY_DISABLED + await using dir = await tmpdir() + const idPath = path.join(dir.path, "machine-id") + fs.writeFileSync(idPath, VALID_UUID, "utf8") + + const encoded = await buildCliContext(idPath) + const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record + + expect(ctx["machine_id"]).toBe(VALID_UUID) }) }) }) describe("getOrCreateMachineId", () => { - test("returns existing id when file is present", () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-")) - const idPath = path.join(tmpDir, "machine-id") - fs.writeFileSync(idPath, "550e8400-e29b-41d4-a716-446655440003\n", "utf8") + test("returns existing id when file is present", async () => { + await using dir = await tmpdir() + const idPath = path.join(dir.path, "machine-id") + fs.writeFileSync(idPath, `${VALID_UUID}\n`, "utf8") - expect(getOrCreateMachineId(idPath)).toBe("550e8400-e29b-41d4-a716-446655440003") - - fs.rmSync(tmpDir, { recursive: true, force: true }) + expect(getOrCreateMachineId(idPath)).toBe(VALID_UUID) }) - test("creates a UUID file when absent and returns it", () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-create-")) - const idPath = path.join(tmpDir, "subdir", "machine-id") + test("creates a UUID file when absent and returns it", async () => { + await using dir = await tmpdir() + const idPath = path.join(dir.path, "subdir", "machine-id") const id = getOrCreateMachineId(idPath) - // Must be a non-empty string that was written to disk expect(typeof id).toBe("string") expect(id.length).toBeGreaterThan(0) expect(fs.readFileSync(idPath, "utf8").trim()).toBe(id) - - fs.rmSync(tmpDir, { recursive: true, force: true }) }) - test("two concurrent callers with absent file converge on the same id", async () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-race-")) - const idPath = path.join(tmpDir, "machine-id") - - // Simulate a race: call getOrCreateMachineId twice before either has written - const [id1, id2] = await Promise.all([ - Promise.resolve(getOrCreateMachineId(idPath)), - Promise.resolve(getOrCreateMachineId(idPath)), - ]) - - expect(id1).toBe(id2) + test("wx EEXIST (lost race) re-reads and returns the winner's id", () => { + const winner = "550e8400-e29b-41d4-a716-4466554400ff" + // Simulate a real race: the file is absent at lstat (ENOENT → create path), + // but a concurrent writer created it before our `wx` write, so writeFileSync + // throws EEXIST and we must re-read what the winner wrote. + const lstatSpy = spyOn(fs, "lstatSync").mockImplementation(() => { + const e = new Error("nope") as NodeJS.ErrnoException + e.code = "ENOENT" + throw e + }) + const mkdirSpy = spyOn(fs, "mkdirSync").mockImplementation(() => undefined as never) + const writeSpy = spyOn(fs, "writeFileSync").mockImplementation(() => { + const e = new Error("exists") as NodeJS.ErrnoException + e.code = "EEXIST" + throw e + }) + const readSpy = spyOn(fs, "readFileSync").mockImplementation(() => winner as never) + try { + expect(getOrCreateMachineId("/does/not/matter/machine-id")).toBe(winner) + expect(readSpy).toHaveBeenCalled() + } finally { + lstatSpy.mockRestore() + mkdirSpy.mockRestore() + writeSpy.mockRestore() + readSpy.mockRestore() + } + }) - fs.rmSync(tmpDir, { recursive: true, force: true }) + test("returns '' when mkdir fails (read-only home) instead of throwing", () => { + // Regression guard: mkdirSync must be inside the try/catch so a read-only + // $HOME / restricted container returns "" rather than propagating and + // breaking sign-in via buildCliContext → buildAuthorizeUrl → authorize(). + const lstatSpy = spyOn(fs, "lstatSync").mockImplementation(() => { + const e = new Error("nope") as NodeJS.ErrnoException + e.code = "ENOENT" + throw e + }) + const mkdirSpy = spyOn(fs, "mkdirSync").mockImplementation(() => { + const e = new Error("eacces") as NodeJS.ErrnoException + e.code = "EACCES" + throw e + }) + try { + expect(getOrCreateMachineId("/root/.altimate/machine-id")).toBe("") + } finally { + lstatSpy.mockRestore() + mkdirSpy.mockRestore() + } }) }) describe("buildAuthorizeUrl", () => { - test("URL contains cli_context param that decodes to valid JSON", async () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-auth-url-")) - const idPath = path.join(tmpDir, "machine-id") - fs.writeFileSync(idPath, "550e8400-e29b-41d4-a716-446655440005", "utf8") - - // Temporarily override machine-id path via a patched buildCliContext call - // by providing a custom machine_id file — buildAuthorizeUrl uses buildCliContext() - // internally with no path arg, so test the URL shape via the exported helper. + let cfg: ReturnType + let savedDisabled: string | undefined + + beforeEach(() => { + savedDisabled = process.env.ALTIMATE_TELEMETRY_DISABLED + delete process.env.ALTIMATE_TELEMETRY_DISABLED + cfg = stubConfig(async () => ({})) + }) + afterEach(() => { + cfg.mockRestore() + if (savedDisabled === undefined) delete process.env.ALTIMATE_TELEMETRY_DISABLED + else process.env.ALTIMATE_TELEMETRY_DISABLED = savedDisabled + }) + + test("carries cli_context in the fragment (not the query), decoding to the machine_id", async () => { + await using dir = await tmpdir() + const idPath = path.join(dir.path, "machine-id") + fs.writeFileSync(idPath, VALID_UUID, "utf8") + + // Forward the temp machine-id path so the test never writes into the real $HOME. const url = await buildAuthorizeUrl( "https://app.myaltimate.com", "http://127.0.0.1:7317/callback", "test-state-abc", + idPath, ) - // cli_context rides in the fragment (#), not the query string expect(url).toContain("#cli_context=") expect(url).toContain("client=altimate-code") expect(url).toContain("state=test-state-abc") expect(url).toContain("redirect=") - // The durable id must NOT be in the query string (it would hit access logs) const parsed = new URL(url) + // The durable id must NOT be in the query string (it would hit access logs). expect(parsed.searchParams.has("cli_context")).toBe(false) - // Extract and decode cli_context from the fragment const encoded = new URLSearchParams(parsed.hash.replace(/^#/, "")).get("cli_context") expect(encoded).toBeTruthy() const ctx = JSON.parse(Buffer.from(encoded!, "base64url").toString("utf8")) as Record expect(ctx["v"]).toBe(1) expect(typeof ctx["cli_version"]).toBe("string") - // machine_id may or may not be present depending on env, but if present must be a string - if (Object.prototype.hasOwnProperty.call(ctx, "machine_id")) { - expect(typeof ctx["machine_id"]).toBe("string") - expect((ctx["machine_id"] as string).length).toBeGreaterThan(0) - } - - fs.rmSync(tmpDir, { recursive: true, force: true }) + // Non-vacuous: the machine_id we wrote must round-trip through the URL. + expect(ctx["machine_id"]).toBe(VALID_UUID) }) - test("deleting cli_context line would cause cli_context to be absent — URL integration is guarded", async () => { - // This test asserts that buildAuthorizeUrl really does embed cli_context in - // the fragment. If the #cli_context=... line were removed, this test fails. - const url = await buildAuthorizeUrl("https://app.myaltimate.com", "http://127.0.0.1:7317/callback", "state-xyz") + test("removing the #cli_context= line would drop the param — integration is guarded", async () => { + await using dir = await tmpdir() + const idPath = path.join(dir.path, "machine-id") + fs.writeFileSync(idPath, VALID_UUID, "utf8") + + const url = await buildAuthorizeUrl( + "https://app.myaltimate.com", + "http://127.0.0.1:7317/callback", + "state-xyz", + idPath, + ) const parsed = new URL(url) expect(new URLSearchParams(parsed.hash.replace(/^#/, "")).has("cli_context")).toBe(true) // And never in the query string, where it would be logged. @@ -196,105 +278,57 @@ describe("buildAuthorizeUrl", () => { }) }) -// altimate_change — additional failure mode tests for getOrCreateMachineId (MINOR 8) +// altimate_change — failure-mode coverage for getOrCreateMachineId describe("getOrCreateMachineId — failure modes", () => { - test("returns empty string for non-UUID content without throwing", () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-invalid-")) - const idPath = path.join(tmpDir, "machine-id") - // Write garbage that is not a v4 UUID + test("returns empty string for non-UUID content without throwing", async () => { + await using dir = await tmpdir() + const idPath = path.join(dir.path, "machine-id") fs.writeFileSync(idPath, "not-a-uuid-at-all", "utf8") - const id = getOrCreateMachineId(idPath) - - // Must return empty string (warn logged internally, not thrown) - expect(id).toBe("") - - fs.rmSync(tmpDir, { recursive: true, force: true }) + expect(getOrCreateMachineId(idPath)).toBe("") }) - test("returns empty string for oversized file without throwing", () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-big-")) - const idPath = path.join(tmpDir, "machine-id") - // Write a file larger than the 512-byte cap + test("returns empty string for oversized file without throwing", async () => { + await using dir = await tmpdir() + const idPath = path.join(dir.path, "machine-id") fs.writeFileSync(idPath, "x".repeat(513), "utf8") - const id = getOrCreateMachineId(idPath) - - // Must return empty string — oversized content rejected - expect(id).toBe("") - - fs.rmSync(tmpDir, { recursive: true, force: true }) + expect(getOrCreateMachineId(idPath)).toBe("") }) - test("returns empty string when file has valid UUID format but wrong version", () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-v1-")) - const idPath = path.join(tmpDir, "machine-id") - // v1 UUID (time-based, not version 4) — third group starts with 1, not 4 + test("returns empty string for a valid UUID of the wrong version", async () => { + await using dir = await tmpdir() + const idPath = path.join(dir.path, "machine-id") + // v1 UUID (third group starts with 1, not 4) fs.writeFileSync(idPath, "550e8400-e29b-11d4-a716-446655440000", "utf8") - const id = getOrCreateMachineId(idPath) - - // Strict UUID v4 validation rejects this - expect(id).toBe("") - - fs.rmSync(tmpDir, { recursive: true, force: true }) + expect(getOrCreateMachineId(idPath)).toBe("") }) - test("returns empty string for an empty file without minting over it", () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-empty-")) - const idPath = path.join(tmpDir, "machine-id") - // 0-byte file exists — must NOT be treated as absent (no exclusive-create mint) + test("returns empty string for an empty file without minting over it", async () => { + await using dir = await tmpdir() + const idPath = path.join(dir.path, "machine-id") fs.writeFileSync(idPath, "", "utf8") - const id = getOrCreateMachineId(idPath) - - // Empty content fails UUID validation → "" (and the file is left untouched) - expect(id).toBe("") + expect(getOrCreateMachineId(idPath)).toBe("") expect(fs.readFileSync(idPath, "utf8")).toBe("") - - fs.rmSync(tmpDir, { recursive: true, force: true }) }) - test("returns empty string when the path is a directory", () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-dir-")) - const idPath = path.join(tmpDir, "machine-id") - // A directory at the machine-id path — lstat rejects it as a non-regular file + test("returns empty string when the path is a directory", async () => { + await using dir = await tmpdir() + const idPath = path.join(dir.path, "machine-id") fs.mkdirSync(idPath) - const id = getOrCreateMachineId(idPath) - - expect(id).toBe("") - - fs.rmSync(tmpDir, { recursive: true, force: true }) + expect(getOrCreateMachineId(idPath)).toBe("") }) - test("returns empty string when the path is a symlink (not followed)", () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-mid-symlink-")) - // Target holds a perfectly valid UUID — lstat must still reject the symlink - // itself rather than following it to read the target. - const targetPath = path.join(tmpDir, "target") + test("returns empty string when the path is a symlink (not followed)", async () => { + await using dir = await tmpdir() + const targetPath = path.join(dir.path, "target") fs.writeFileSync(targetPath, "550e8400-e29b-41d4-a716-446655440099", "utf8") - const linkPath = path.join(tmpDir, "machine-id") + const linkPath = path.join(dir.path, "machine-id") fs.symlinkSync(targetPath, linkPath) - const id = getOrCreateMachineId(linkPath) - - expect(id).toBe("") - - fs.rmSync(tmpDir, { recursive: true, force: true }) - }) - - test("buildCliContext omits machine_id when file contains non-UUID content", async () => { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "altimate-ctx-corrupt-")) - const idPath = path.join(tmpDir, "machine-id") - fs.writeFileSync(idPath, "not-a-uuid", "utf8") - - const encoded = await buildCliContext(idPath) - const ctx = JSON.parse(Buffer.from(encoded, "base64url").toString("utf8")) as Record - - // machine_id must be absent — non-UUID content is rejected - expect(Object.prototype.hasOwnProperty.call(ctx, "machine_id")).toBe(false) - - fs.rmSync(tmpDir, { recursive: true, force: true }) + expect(getOrCreateMachineId(linkPath)).toBe("") }) }) From 346df2cdee7bb49fbc470e29003a13d8baad2d67 Mon Sep 17 00:00:00 2001 From: Sarav Date: Fri, 7 Aug 2026 12:50:39 +0530 Subject: [PATCH 6/6] =?UTF-8?q?fix:=20[AI]=20round-4=20review=20=E2=80=94?= =?UTF-8?q?=20bounded=20read,=20fail-closed=20log/comment,=20docs=20wordin?= =?UTF-8?q?g?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - machine-id: read at most MAX_BYTES through a descriptor (fstat + readSync) so the size cap is enforced at read time rather than advisory; covers the EEXIST race re-read too (previously an unbounded readFileSync) - buildCliContext: log the fail-closed config-unreadable branch and correct the comment — Config.get() resolves in a normal browser authorize() (server routes run inside Instance.provide); the known throw is `auth login `, which skips instance bootstrap - welcome.ts: correct the minting comment — doInit is the owner, but its early (pre-Instance) call fails open on the config gate, a pre-existing telemetry-init gap tracked separately; this file just stops adding a second env-only minting site - docs: describe the machine id as a device/installation identifier (persisted and reused across sessions) and drop the "never used for tracking" overclaim Co-Authored-By: Claude Opus 4.8 --- docs/docs/reference/telemetry.md | 2 +- .../opencode/src/altimate/plugin/altimate.ts | 20 ++++++++----- .../opencode/src/altimate/util/machine-id.ts | 29 ++++++++++++++++--- packages/opencode/src/cli/welcome.ts | 12 ++++---- .../test/altimate/altimate-plugin.test.ts | 13 ++++++++- 5 files changed, 58 insertions(+), 18 deletions(-) diff --git a/docs/docs/reference/telemetry.md b/docs/docs/reference/telemetry.md index d7c52706b..c092eeb19 100644 --- a/docs/docs/reference/telemetry.md +++ b/docs/docs/reference/telemetry.md @@ -151,7 +151,7 @@ Both identifiers are only sent when telemetry is enabled. Disable telemetry enti ### CLI Authentication Flow -When you sign in using the CLI browser auth flow (`altimate auth login`), an anonymized session identifier (the `machine-id` UUID) is included in the authorization URL and associated with your account in product analytics. This is used solely to correlate CLI install events with authenticated accounts in aggregate funnel analytics — it is never used for tracking, advertising, or cross-site identification. Your telemetry opt-out suppresses this: when you disable telemetry — via `ALTIMATE_TELEMETRY_DISABLED=true` **or** the `telemetry.disabled` config option — the machine ID is omitted from the authorization URL entirely. The machine ID is associated with your account in PostHog for this funnel analysis, separate from the Azure Application Insights pipeline used for other CLI telemetry events. +When you sign in using the CLI browser auth flow (`altimate auth login`), the anonymous machine ID (a random UUID persisted at `~/.altimate/machine-id` — a device/installation identifier, reused across sessions) is included in the authorization URL and associated with your account in product analytics. This is used solely to correlate CLI install events with authenticated accounts in aggregate funnel analytics — it is not used for advertising or cross-site tracking. Your telemetry opt-out suppresses this: when you disable telemetry — via `ALTIMATE_TELEMETRY_DISABLED=true` **or** the `telemetry.disabled` config option — the machine ID is omitted from the authorization URL entirely. The machine ID is associated with your account in PostHog for this funnel analysis, separate from the Azure Application Insights pipeline used for other CLI telemetry events. ### Data Retention diff --git a/packages/opencode/src/altimate/plugin/altimate.ts b/packages/opencode/src/altimate/plugin/altimate.ts index 059b89e76..b57b4233e 100644 --- a/packages/opencode/src/altimate/plugin/altimate.ts +++ b/packages/opencode/src/altimate/plugin/altimate.ts @@ -83,13 +83,19 @@ export async function buildCliContext(machineIdPath?: string): Promise { try { const userConfig = (await Config.get()) as any disabled = Boolean(userConfig.telemetry?.disabled) - } catch { - // Config unreadable here — this plugin can run in the server worker where - // Config.get() throws "InstanceRef not provided". Fail CLOSED: omit the - // durable machine_id. A missed correlation is preferable to transmitting a - // stable cross-session device identifier for a user who may have opted out - // via config. (Intentionally stricter than doInit's fail-open, which - // governs single events rather than a persistent identifier.) + } catch (err) { + // Config was unreadable — NOT the normal path. Server routes run inside + // Instance.provide() (AsyncLocalStorage-propagated across awaits), so + // Config.get() resolves during an ordinary browser authorize(). The known + // exception is `altimate auth login `, which deliberately skips + // instance bootstrap (ProvidersLoginCommand `instance: (args) => !args.url`); + // on that path this fires. Fail CLOSED — omit the durable machine_id rather + // than transmit it for a user who may have opted out via config; a missed + // correlation beats leaking a stable identifier. Log so a low correlation + // rate is traceable here instead of being mistaken for a lost reply. + log.warn("cli_context: config unreadable, omitting machine_id (fail-closed)", { + code: (err as NodeJS.ErrnoException)?.code, + }) disabled = true } } diff --git a/packages/opencode/src/altimate/util/machine-id.ts b/packages/opencode/src/altimate/util/machine-id.ts index 5b6ff1d39..30956ceb0 100644 --- a/packages/opencode/src/altimate/util/machine-id.ts +++ b/packages/opencode/src/altimate/util/machine-id.ts @@ -17,6 +17,26 @@ const MAX_BYTES = 512 // RFC 4122 v4 UUID — the only format we mint, so the only format we accept. const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i +// Read at most MAX_BYTES through a descriptor so the cap is ENFORCED at read +// time rather than being advisory: a plain readFileSync would slurp a file that +// grew or was swapped after an earlier lstat entirely into memory before UUID_RE +// could reject it. `fstat` on the open descriptor re-confirms a regular file of +// bounded size — the same file we then read — so both the main path and the +// EEXIST race re-read (which previously had no check at all) are covered. +// Returns "" for anything not a bounded regular file; callers treat that as invalid. +function readCappedUtf8(idPath: string): string { + const fd = fs.openSync(idPath, "r") + try { + const stat = fs.fstatSync(fd) + if (!stat.isFile() || stat.size > MAX_BYTES) return "" + const buf = Buffer.alloc(MAX_BYTES) + const bytesRead = fs.readSync(fd, buf, 0, MAX_BYTES, 0) + return buf.toString("utf8", 0, bytesRead).trim() + } finally { + fs.closeSync(fd) + } +} + /** * Read the machine-id from `~/.altimate/machine-id`, minting a new random UUID * with `flag: "wx"` (exclusive create) if the file is absent. @@ -25,7 +45,8 @@ const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f * same UUID — the winner writes, the loser re-reads. * - **Regular-file-only**: uses `lstat` and rejects symlinks / non-regular * files rather than following them to an attacker-chosen target. - * - **Size-capped**: reads at most 512 bytes to avoid multi-MB file attacks. + * - **Size-capped**: reads at most 512 bytes through a descriptor (enforced at + * read time, not an advisory pre-check) to avoid multi-MB file reads. * - **UUID-validated**: rejects content that does not match RFC 4122 v4 UUID * format (corrupt file, injected content, etc.) and returns `""` with a warn * log so callers can omit the field rather than propagate garbage. @@ -51,7 +72,7 @@ export function getOrCreateMachineId(machineIdPath?: string): string { log.warn("machine-id file exceeds size limit — omitting", { path: idPath, size: stat.size }) return "" } - raw = fs.readFileSync(idPath, "utf8").trim() + raw = readCappedUtf8(idPath) } catch (readErr) { const code = (readErr as NodeJS.ErrnoException)?.code if (code !== "ENOENT") { @@ -91,9 +112,9 @@ export function getOrCreateMachineId(machineIdPath?: string): string { log.warn("machine-id create failed", { code, path: idPath }) return "" } - // Lost the race — read what the winner wrote. + // Lost the race — read what the winner wrote (bounded, same as the main read). try { - const winner = fs.readFileSync(idPath, "utf8").trim() + const winner = readCappedUtf8(idPath) if (!UUID_RE.test(winner)) { log.warn("machine-id written by race winner is non-UUID — omitting", { path: idPath }) return "" diff --git a/packages/opencode/src/cli/welcome.ts b/packages/opencode/src/cli/welcome.ts index 84a20405d..650a851a3 100644 --- a/packages/opencode/src/cli/welcome.ts +++ b/packages/opencode/src/cli/welcome.ts @@ -40,11 +40,13 @@ export function showWelcomeBannerIfNeeded(): void { fs.unlinkSync(markerPath) // altimate_change start — "upgrade" means the machine-id file already existed before this - // launch. Probe existence with existsSync only — do NOT mint here. Minting is owned by - // Telemetry.doInit(), which resolves the full opt-out policy (env var AND config) before - // creating the file; minting here would duplicate that decision under a weaker gate and - // create the id for a config-opted-out user. The first_launch machine_id is attached at - // flush time from telemetry module state, so it does not depend on minting here. + // launch. Probe existence with existsSync only — do NOT mint here. Minting is left to + // Telemetry.doInit() (its job, not the welcome banner's); the first_launch machine_id is + // attached at flush time from telemetry module state, so it does not depend on minting here. + // NOTE: doInit's early call runs before an Instance is available, so its CONFIG opt-out gate + // fails open and it can still mint for a config-only opt-out user. That is a pre-existing + // telemetry-init gap (tracked separately), not something this banner can fix — this code just + // stops adding a SECOND minting site under an even weaker (env-only) gate. const machineIdPath = path.join(os.homedir(), ".altimate", "machine-id") const isUpgrade = fs.existsSync(machineIdPath) // altimate_change end diff --git a/packages/opencode/test/altimate/altimate-plugin.test.ts b/packages/opencode/test/altimate/altimate-plugin.test.ts index 7bdcbde9a..c6dbb791c 100644 --- a/packages/opencode/test/altimate/altimate-plugin.test.ts +++ b/packages/opencode/test/altimate/altimate-plugin.test.ts @@ -178,7 +178,15 @@ describe("getOrCreateMachineId", () => { e.code = "EEXIST" throw e }) - const readSpy = spyOn(fs, "readFileSync").mockImplementation(() => winner as never) + // The re-read goes through the bounded, descriptor-based readCappedUtf8: + // open → fstat (regular file, bounded) → read ≤ MAX_BYTES → close. + const openSpy = spyOn(fs, "openSync").mockImplementation(() => 3 as never) + const fstatSpy = spyOn(fs, "fstatSync").mockImplementation( + () => ({ isFile: () => true, size: winner.length }) as never, + ) + const readSpy = spyOn(fs, "readSync").mockImplementation(((_fd: number, buf: Buffer) => + Buffer.from(winner).copy(buf)) as never) + const closeSpy = spyOn(fs, "closeSync").mockImplementation(() => undefined as never) try { expect(getOrCreateMachineId("/does/not/matter/machine-id")).toBe(winner) expect(readSpy).toHaveBeenCalled() @@ -186,7 +194,10 @@ describe("getOrCreateMachineId", () => { lstatSpy.mockRestore() mkdirSpy.mockRestore() writeSpy.mockRestore() + openSpy.mockRestore() + fstatSpy.mockRestore() readSpy.mockRestore() + closeSpy.mockRestore() } })