Skip to content

Commit ffe8b1f

Browse files
saravmajesticclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 43fb343 commit ffe8b1f

6 files changed

Lines changed: 244 additions & 194 deletions

File tree

docs/docs/reference/security-faq.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,9 @@ export ALTIMATE_TELEMETRY_DISABLED=true
130130

131131
- **Logged-in users:** Your email is SHA-256 hashed before sending. We never see your raw email.
132132
- **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.
133-
- **Both identifiers** are only sent when telemetry is enabled. Disable with `ALTIMATE_TELEMETRY_DISABLED=true`.
133+
- **Both identifiers** are only sent when telemetry is enabled. Disable via `ALTIMATE_TELEMETRY_DISABLED=true` or the `telemetry.disabled` config option.
134134
- **No fingerprinting:** We do not use browser fingerprinting, hardware IDs, MAC addresses, or IP-based tracking.
135-
- **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.
135+
- **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.
136136

137137
### What happens on first launch?
138138

docs/docs/reference/telemetry.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ Both identifiers are only sent when telemetry is enabled. Disable telemetry enti
151151

152152
### CLI Authentication Flow
153153

154-
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.
154+
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.
155155

156156
### Data Retention
157157

packages/opencode/src/altimate/plugin/altimate.ts

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -54,37 +54,43 @@ const DEFAULT_API_URL = "https://api.myaltimate.com"
5454

5555
const log = Log.create({ service: "altimate-plugin" })
5656

57-
// altimate_change — getOrCreateMachineId is now in util/machine-id.ts (re-exported
58-
// from there so existing test imports that reference this module continue to work).
59-
export { getOrCreateMachineId } from "../util/machine-id"
60-
6157
// Builds a base64url-encoded context blob for correlating this browser auth
6258
// session with CLI telemetry in PostHog. The machine_id is a random UUID
6359
// stored at ~/.altimate/machine-id — not tied to hardware, OS, or user identity.
64-
// After sign-in, the frontend calls posthog.alias(email, machine_id) to associate
65-
// the device with the authenticated account in product analytics.
60+
// After sign-in the frontend registers it as the `cli_machine_id` PostHog
61+
// super-property so the CLI device is attributed to the authenticated account
62+
// in aggregate funnel analytics.
6663
//
6764
// Privacy note: cli_context is sent in the URL *fragment* (#cli_context=...),
6865
// not the query string. The browser never transmits a fragment to the server,
6966
// so the machine_id — though a non-PII crypto.randomUUID() — stays out of
7067
// app.myaltimate.com's access logs, any fronting CDN/WAF, and the Referer
7168
// header, while remaining readable by the /register page via location.hash.
72-
// The frontend reads it from the fragment (see useCliContext.ts). The fragment
73-
// must be the last URL segment, after all query params.
69+
//
70+
// Frontend decode contract (implemented in monorepo useCliContext.ts /
71+
// cliContext.ts): the value is base64url (not standard base64); the consumer
72+
// must catch decode/JSON errors, require `v === 1`, validate that `machine_id`
73+
// and `cli_version` are strings, treat `cli_version: "local"` as a valid dev
74+
// build, and treat the payload as untrusted (anyone can craft a URL). An absent
75+
// `machine_id` means "do not attribute" — it must never be aliased on.
7476
export async function buildCliContext(machineIdPath?: string): Promise<string> {
7577
// altimate_change start — honour both telemetry opt-out gates, mirroring
76-
// telemetry/index.ts::doInit exactly:
77-
// 1. ALTIMATE_TELEMETRY_DISABLED=true env var (early, always-works escape hatch)
78+
// telemetry/index.ts::doInit:
79+
// 1. ALTIMATE_TELEMETRY_DISABLED=true env var (always-works hard opt-out)
7880
// 2. config.telemetry.disabled (resolved via the async Config.get())
79-
// Config.get() may throw outside an Instance context; treat a config failure as
80-
// "not disabled" (same as doInit) — the env var above is the hard opt-out.
8181
let disabled = process.env.ALTIMATE_TELEMETRY_DISABLED === "true"
8282
if (!disabled) {
8383
try {
8484
const userConfig = (await Config.get()) as any
8585
disabled = Boolean(userConfig.telemetry?.disabled)
8686
} catch {
87-
// Config unavailable — proceed with telemetry enabled.
87+
// Config unreadable here — this plugin can run in the server worker where
88+
// Config.get() throws "InstanceRef not provided". Fail CLOSED: omit the
89+
// durable machine_id. A missed correlation is preferable to transmitting a
90+
// stable cross-session device identifier for a user who may have opted out
91+
// via config. (Intentionally stricter than doInit's fail-open, which
92+
// governs single events rather than a persistent identifier.)
93+
disabled = true
8894
}
8995
}
9096
let machineId = ""
@@ -94,25 +100,32 @@ export async function buildCliContext(machineIdPath?: string): Promise<string> {
94100
machineId = getOrCreateMachineId(machineIdPath)
95101
}
96102
// altimate_change end
97-
// altimate_change start — omit machine_id key when empty (matches telemetry
98-
// module pattern: `...(machineId && { machine_id: machineId })`). Sending ""
99-
// is meaningless for posthog.alias() and misleads downstream consumers.
103+
// altimate_change start — omit machine_id when empty (matches the telemetry
104+
// module's `...(machineId && { machine_id })`). An empty value is meaningless
105+
// to the frontend super-property registration and must not be sent.
100106
const ctx: Record<string, unknown> = { v: 1, cli_version: InstallationVersion }
101107
if (machineId) ctx.machine_id = machineId
102108
// altimate_change end
103109
return Buffer.from(JSON.stringify(ctx)).toString("base64url")
104110
}
105111

106112
// altimate_change start — exported so tests can assert on the full URL shape
107-
// without duplicating the construction logic.
108-
export async function buildAuthorizeUrl(webUrl: string, redirect: string, state: string): Promise<string> {
113+
// without duplicating the construction logic. `machineIdPath` is forwarded to
114+
// buildCliContext so tests can point at a temp file instead of writing a real
115+
// id into the runner's $HOME.
116+
export async function buildAuthorizeUrl(
117+
webUrl: string,
118+
redirect: string,
119+
state: string,
120+
machineIdPath?: string,
121+
): Promise<string> {
109122
return (
110123
`${webUrl}/register?client=altimate-code` +
111124
`&redirect=${encodeURIComponent(redirect)}` +
112-
`&state=${state}` +
125+
`&state=${encodeURIComponent(state)}` +
113126
// Fragment (#), not a query param — keeps the durable machine_id out of
114127
// server access logs / Referer. Must stay last, after all query params.
115-
`#cli_context=${encodeURIComponent(await buildCliContext())}`
128+
`#cli_context=${encodeURIComponent(await buildCliContext(machineIdPath))}`
116129
)
117130
}
118131
// altimate_change end

packages/opencode/src/altimate/util/machine-id.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,14 @@ export function getOrCreateMachineId(machineIdPath?: string): string {
7575
// --- Create path (ENOENT) ---
7676
// `flag: "wx"` is atomic exclusive-create: the OS guarantees only one writer
7777
// succeeds. The loser re-reads what the winner wrote.
78+
//
79+
// mkdirSync MUST be inside this try: on a read-only $HOME, a restricted
80+
// container, or a full disk it throws (EACCES/EROFS/ENOSPC), and the module's
81+
// contract is to return "" on every error — never propagate. The auth path
82+
// (buildCliContext) relies on this and no longer wraps the call itself.
7883
const candidate = randomUUID()
79-
fs.mkdirSync(path.dirname(idPath), { recursive: true })
8084
try {
85+
fs.mkdirSync(path.dirname(idPath), { recursive: true })
8186
fs.writeFileSync(idPath, candidate, { encoding: "utf8", flag: "wx" })
8287
return candidate
8388
} catch (writeErr) {

packages/opencode/src/cli/welcome.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import { EOL } from "os"
66
// altimate_change start — import Telemetry for first_launch event
77
import { Telemetry } from "../altimate/telemetry"
88
// altimate_change end
9-
// altimate_change — import shared machine-id utility so the path is canonical across all call sites
10-
import { getOrCreateMachineId } from "../altimate/util/machine-id"
119

1210
const APP_NAME = "altimate-code"
1311
const MARKER_FILE = ".installed-version"
@@ -42,13 +40,13 @@ export function showWelcomeBannerIfNeeded(): void {
4240
fs.unlinkSync(markerPath)
4341

4442
// altimate_change start — "upgrade" means the machine-id file already existed before this
45-
// launch. Probe existence with existsSync FIRST — do NOT use getOrCreateMachineId() as the
46-
// probe, because it mints the file on a fresh install and would then report every new user
47-
// as an upgrade. After probing, mint the id (unless telemetry is opted out via env) so
48-
// telemetry.doInit() finds it ready — welcome.ts runs before doInit().
43+
// launch. Probe existence with existsSync only — do NOT mint here. Minting is owned by
44+
// Telemetry.doInit(), which resolves the full opt-out policy (env var AND config) before
45+
// creating the file; minting here would duplicate that decision under a weaker gate and
46+
// create the id for a config-opted-out user. The first_launch machine_id is attached at
47+
// flush time from telemetry module state, so it does not depend on minting here.
4948
const machineIdPath = path.join(os.homedir(), ".altimate", "machine-id")
5049
const isUpgrade = fs.existsSync(machineIdPath)
51-
if (process.env.ALTIMATE_TELEMETRY_DISABLED !== "true") getOrCreateMachineId()
5250
// altimate_change end
5351

5452
// altimate_change start — track first launch for new user counting (privacy-safe: only version + machine_id)

0 commit comments

Comments
 (0)