Skip to content

Stamp anonymous X-Gusto-CLI-Install-Id header on outbound requests - #153

Open
DamoneMX wants to merge 1 commit into
mainfrom
install-id-header
Open

Stamp anonymous X-Gusto-CLI-Install-Id header on outbound requests#153
DamoneMX wants to merge 1 commit into
mainfrom
install-id-header

Conversation

@DamoneMX

Copy link
Copy Markdown
Contributor

Summary

Threads an anonymous per-install UUID through every outbound HTTP request the CLI makes (REST + MCP + OAuth), so the server-side request log can join pre-login DCR + token exchange events to post-login authed activity via install_id. Server side reads the header off the standard request payload; no new endpoint or dedicated telemetry option is introduced.

The header is X-Gusto-CLI-Install-Id, value is a UUIDv4 persisted once per install to ~/.config/gusto/config.toml. Opt-out via GUSTO_TELEMETRY=0 (or false/no) suppresses generation and header stamping entirely. A follow-up ticket will cover socializing the opt-out (README section + gusto auth login --help note) so users can discover it.

Design decisions

Atomic config writes. writeConfig was in-place Bun.write before; a concurrent reader could observe a half-written file and blow up parse(). Switched to write-to-temp + POSIX rename (atomic on the same filesystem), mirroring the pattern already used by src/lib/oauth/token-store.ts for the OAuth credentials file. Per-call temp filename includes randomUUID() so same-process concurrent writes don't collide on the tmp path (surfaced by a concurrency test during development). Best-effort rm(tmp) cleanup on failure prevents orphan .tmp files in the user's config dir.

Fail-open on the hot path. resolveInstallIdHeader runs on the hot path of every REST + MCP + OAuth request via resolveApiContext, callMcpTool, and oauthHttp. If the config file becomes unreadable/unwritable (bad TOML, permissions, ENOSPC), the try/catch returns undefined so telemetry silently degrades and the user's command keeps working. Telemetry never breaks the CLI.

Per-process memoization. Resolved once per invocation on the default-paths path, so the config file is read at most once regardless of how many outbound requests a single command makes. Tests that pass explicit paths bypass the cache to get fresh resolution.

First-run race is bounded and self-heals. Two racing callers of getOrCreateInstallId may each generate + write their own UUID; last-writer-wins on disk and every future caller converges on that value. Full first-writer-wins would need a lock file; accepted as a bounded, self-healing divergence rather than adding lock machinery for a race that resolves in one command.

Corruption regenerates. pickValid shape-checks install_id against a UUID pattern. A hand-edited or corrupted value on disk is dropped on read, and getOrCreateInstallId regenerates a fresh one on the next call. Sibling config keys are preserved through the drop-and-regenerate.

Opt-out has zero filesystem side effects. Under GUSTO_TELEMETRY=0 on a fresh install, no config directory is created and no UUID is generated. The header is omitted from outbound requests entirely, not sent as an empty value.

gusto config set preserves install_id. configSetHandler now resolves configPaths() once and threads it through readConfig + writeConfig so the two can't drift, and existing users setting format or environment don't clobber their install_id.

Where the header is stamped: authed REST + MCP calls in src/lib/api-client.ts sendOnce; pre-auth DCR (postJson) and token exchange (postForm) in src/lib/oauth/endpoints.ts via a withInstallIdHeader helper.

Test plan

  • bun run typecheck — clean.
  • bun run lint — clean.
  • bun test — 1205 pass / 1 fail. The one failure (licenses.test.ts --check exits 0 when NOTICES is current) is pre-existing on origin/main, unrelated to this change.
  • bun run build — 77 modules, compiles clean.
  • Local smoke (fresh XDG_CONFIG_HOME): first CLI invocation writes install_id to config.toml; second reuses the same value; the outbound request carries X-Gusto-CLI-Install-Id: <uuid> (verified via unit-test fetch mock).
  • Opt-out smoke: GUSTO_TELEMETRY=0 on a fresh dir → no gusto/ subdirectory created (zero filesystem side effects) and no header on outbound requests.
  • Corruption self-heal smoke: install_id = "not-a-uuid" on disk alongside format = "agent" → CLI regenerates a fresh UUID while preserving format = "agent".
  • Concurrency test: Promise.all([getOrCreateInstallId, getOrCreateInstallId]) in the same process produces valid UUIDs and the file converges to one of them.
  • Fail-open test: unwritable config path → resolveInstallIdHeader returns undefined and the CLI continues without the header.

@DamoneMX
DamoneMX requested review from a team and ashieh as code owners July 28, 2026 23:32
Threads an anonymous per-install UUID through every outbound HTTP request
the CLI makes (REST + MCP + OAuth), so the server-side request log can
join pre-login DCR + token exchange events to post-login authed activity
via `install_id`. Server side reads the header off the standard request
payload; no new endpoint or dedicated telemetry option is introduced.

## What changed

- `src/lib/config.ts`
  - Add `install_id` to `UserConfig`, generated once per install by
    `getOrCreateInstallId` and persisted to `~/.config/gusto/config.toml`.
    Intentionally not user-configurable through `gusto config set`.
  - `writeConfig` now writes atomically (write-to-temp + POSIX `rename`).
    Per-call temp filename includes `randomUUID()` so same-process
    concurrent writes cannot collide. Best-effort `rm(tmp)` cleanup on
    failure so a failed write can't leave a stray `.tmp` behind.
  - `pickValid` shape-checks `install_id` against a UUID pattern so a
    corrupted or hand-edited value falls back to fresh generation
    instead of shipping garbage as the join key.
  - `resolveInstallIdHeader` memoizes the resolved Promise per-process
    on the default-paths path. Fail-open: any I/O error returns
    undefined so telemetry never breaks the user's command.
- `src/lib/env.ts` — `isTelemetryEnabled` reads `GUSTO_TELEMETRY`;
  opt-out on `0`/`false`/`no` (case-insensitive).
- `src/lib/api-client.ts` — `installId` option on `ApiClientOptions`,
  stamped as `X-Gusto-CLI-Install-Id` in `sendOnce`. Header omitted
  (not sent empty) when undefined.
- `src/lib/oauth/endpoints.ts` — `installId` on `OAuthHttpOptions`.
  `withInstallIdHeader` stamps the header on `postJson` (DCR) and
  `postForm` (token exchange) so pre-auth events carry the join key.
- `src/lib/oauth/context.ts` — `oauthHttp` is now async and resolves
  install_id via `resolveInstallIdHeader`. `oauthApiClient` threads
  the value through to its ApiClient.
- `src/lib/api-context.ts` — `buildApiClient` accepts and threads
  `installId`. `resolveApiContext` resolves it before building the
  client. `sessionToken` awaits `oauthHttp`.
- `src/lib/mcp.ts` — `callMcpTool` resolves and threads `installId`
  into the MCP ApiClient.
- `src/commands/auth.ts` — `authLoginHandler` awaits `oauthHttp`.
- `src/commands/config.ts` — `configSetHandler` resolves `configPaths()`
  once and threads it through `readConfig` and `writeConfig` so the two
  can't drift.

## Design notes

- **First-run race:** two racing callers of `getOrCreateInstallId` may
  each generate + write their own UUID. Last-writer-wins on disk; every
  future caller converges on that value. Full first-writer-wins would
  need a lock file; accepted as a bounded, self-healing divergence.
- **Opt-out is silent:** `GUSTO_TELEMETRY=0` suppresses install_id
  resolution entirely — no config file written, zero filesystem side
  effects.
- **No PII pre-auth:** the header carries only the anonymous UUID.
  Post-auth events pick up `resource_owner_id` + `company_uuid` via
  the existing OAuth attribution on the payload.

## Test plan

- [x] `bun run typecheck` — clean.
- [x] `bun run lint` — clean.
- [x] `bun test` — 1205 pass / 1 fail. The one failure
  (`licenses.test.ts --check exits 0 when NOTICES is current`) is
  pre-existing on `origin/main`, unrelated to this change.
- [x] `bun run build` — 77 modules, compiles clean.
- [x] Local smoke (fresh `XDG_CONFIG_HOME`): first CLI invocation
  writes install_id to `config.toml`; second reuses the same value.
- [x] Opt-out smoke: `GUSTO_TELEMETRY=0` on a fresh dir → no `gusto/`
  subdirectory created (zero filesystem side effects).
- [x] Corruption self-heal smoke: `install_id = "not-a-uuid"` on disk
  alongside `format = "agent"` → CLI regenerates a fresh UUID while
  preserving `format = "agent"`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Arie Radilla <arie.radillalaureano@gusto.com>
@DamoneMX
DamoneMX force-pushed the install-id-header branch from 9863465 to 62be8e1 Compare July 28, 2026 23:40

@jeff-gusto jeff-gusto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two small suggestions — the atomic-write change is right, this is just tightening it to match the precedent it cites.

Comment thread src/lib/config.ts
Comment on lines +66 to +76
const { mkdir, chmod, rename, rm } = await import("node:fs/promises");
await mkdir(paths.dir, { recursive: true, mode: 0o700 });
await Bun.write(paths.file, stringify(stripUndefined(cfg)));
await chmod(paths.file, 0o600);
// Write to a uniquely-named temp file and rename into place: POSIX rename on the same
// filesystem is atomic, so a concurrent reader can never observe a half-written file.
// Suffix includes pid + a UUID so two concurrent writes in the same process (or across
// processes with recycled PIDs) don't step on each other's temp file.
const tmp = `${paths.file}.${process.pid}.${randomUUID()}.tmp`;
try {
await Bun.write(tmp, stringify(stripUndefined(cfg)));
await chmod(tmp, 0o600);
await rename(tmp, paths.file);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The commit message says this mirrors token-store.ts, but that one sets the mode at creation time (writeFile(tmp, data, { mode: 0o600 })) specifically so the file is never briefly readable at the umask default. Bun.write + a separate chmod keeps that window open — it just moves it from the final path onto the temp file. Low stakes here since config.toml holds no secrets, but it's a free fix and keeps the two writers consistent.

Suggested change
const { mkdir, chmod, rename, rm } = await import("node:fs/promises");
await mkdir(paths.dir, { recursive: true, mode: 0o700 });
await Bun.write(paths.file, stringify(stripUndefined(cfg)));
await chmod(paths.file, 0o600);
// Write to a uniquely-named temp file and rename into place: POSIX rename on the same
// filesystem is atomic, so a concurrent reader can never observe a half-written file.
// Suffix includes pid + a UUID so two concurrent writes in the same process (or across
// processes with recycled PIDs) don't step on each other's temp file.
const tmp = `${paths.file}.${process.pid}.${randomUUID()}.tmp`;
try {
await Bun.write(tmp, stringify(stripUndefined(cfg)));
await chmod(tmp, 0o600);
await rename(tmp, paths.file);
const { mkdir, writeFile, rename, rm } = await import("node:fs/promises");
await mkdir(paths.dir, { recursive: true, mode: 0o700 });
// Write to a uniquely-named temp file and rename into place: POSIX rename on the same
// filesystem is atomic, so a concurrent reader can never observe a half-written file.
// Suffix includes pid + a UUID so two concurrent writes in the same process (or across
// processes with recycled PIDs) don't step on each other's temp file.
const tmp = `${paths.file}.${process.pid}.${randomUUID()}.tmp`;
try {
// Mode at creation, not a follow-up chmod: never expose the file at the umask default.
await writeFile(tmp, stringify(stripUndefined(cfg)), { mode: 0o600 });
await rename(tmp, paths.file);

Comment thread src/lib/config.test.ts
await Bun.write(paths.file, `install_id = "not-a-uuid"\nformat = "agent"\n`);
// Invalid install_id is dropped; sibling valid keys are preserved.
expect(await readConfig(paths)).toEqual({ format: "agent" });
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The /i on INSTALL_ID_PATTERN is the only thing letting an uppercase-hex UUID on disk survive pickValid, but nothing exercises it — every test here uses lowercase-valid or outright-garbage values. Drop the flag in a future refactor and CI stays green while hand-edited or migrated configs silently regenerate their id on every run.

Suggested change
});
});
test("readConfig keeps an uppercase-hex install_id from disk", async () => {
const upper = "3F2504E0-4F89-41D3-9A0C-0305E82C3301";
await Bun.write(paths.file, `install_id = "${upper}"\n`);
expect(await readConfig(paths)).toEqual({ install_id: upper });
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants