Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/commands/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ export function authLoginHandler(
try {
const info = await doLogin(defaultEnv(globals.env), {
store: resolveStore(),
http: oauthHttp(globals),
http: await oauthHttp(globals),
noBrowser: opts.noBrowser,
emitEvent: buildSignInUrlEmitter(globals, sinks),
});
Expand Down
5 changes: 3 additions & 2 deletions src/commands/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,10 @@ function configSetHandler(key: string, value: string): CommandHandler {
};
}
const normalized = normalizeValue(validKey, value);
const cfg = await readConfig();
const paths = configPaths();
const cfg = await readConfig(paths);
const updated: UserConfig = { ...cfg, [validKey]: normalized };
await writeConfig(updated);
await writeConfig(updated, paths);
return { ok: true, data: { key: validKey, value: normalized } };
};
}
Expand Down
18 changes: 18 additions & 0 deletions src/lib/api-client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,24 @@ describe("ApiClient basics", () => {
expect(result.status).toBe(200);
});

test("stamps X-Gusto-CLI-Install-Id when installId is configured", async () => {
const captured: { url?: string; init?: RequestInit } = {};
const client = makeClient(mockFetch(captured, { status: 200, body: {} }), {
installId: "11111111-2222-4333-8444-555555555555",
});
await client.get("/v1/me");
const headers = captured.init?.headers as Record<string, string>;
expect(headers["X-Gusto-CLI-Install-Id"]).toBe("11111111-2222-4333-8444-555555555555");
});

test("omits X-Gusto-CLI-Install-Id entirely when installId is undefined (opt-out)", async () => {
const captured: { url?: string; init?: RequestInit } = {};
const client = makeClient(mockFetch(captured, { status: 200, body: {} }));
await client.get("/v1/me");
const headers = captured.init?.headers as Record<string, string>;
expect(headers["X-Gusto-CLI-Install-Id"]).toBeUndefined();
});

test("POST sends a JSON body and the right content-type", async () => {
const captured: { url?: string; init?: RequestInit } = {};
const client = makeClient(mockFetch(captured, { status: 201, body: { id: "x" } }));
Expand Down
7 changes: 7 additions & 0 deletions src/lib/api-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ export interface ApiClientOptions {
baseUrl: string;
token: string;
apiVersion: string;
/** Anonymous per-install UUID sent as `X-Gusto-CLI-Install-Id`; omit to suppress the header. */
installId?: string;
fetchImpl?: typeof fetch;
timeoutMs?: number;
maxRetries?: number;
Expand Down Expand Up @@ -142,6 +144,7 @@ export class ApiClient {
private readonly baseUrl: string;
private readonly token: string;
private readonly apiVersion: string;
private readonly installId?: string;
private readonly fetchImpl: typeof fetch;
private readonly timeoutMs: number;
private readonly maxRetries: number;
Expand All @@ -152,6 +155,7 @@ export class ApiClient {
this.baseUrl = opts.baseUrl.replace(/\/$/, "");
this.token = opts.token;
this.apiVersion = opts.apiVersion;
this.installId = opts.installId;
this.fetchImpl = opts.fetchImpl ?? fetch;
this.timeoutMs = opts.timeoutMs ?? DEFAULT_TIMEOUT_MS;
this.maxRetries = opts.maxRetries ?? DEFAULT_MAX_RETRIES;
Expand Down Expand Up @@ -350,6 +354,9 @@ export class ApiClient {
Accept: "application/json",
"X-Gusto-API-Version": this.apiVersion,
};
if (this.installId !== undefined) {
headers["X-Gusto-CLI-Install-Id"] = this.installId;
}
let init: RequestInit = { method, headers, signal: AbortSignal.timeout(timeoutMs) };
if (body !== undefined) {
headers["Content-Type"] = "application/json";
Expand Down
9 changes: 6 additions & 3 deletions src/lib/api-context.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ApiClient, stderrRequestObserver } from "./api-client.ts";
import { resolveInstallIdHeader } from "./config.ts";
import { confirmationGate } from "./confirm.ts";
import { defaultEnv, getAccessToken, getCompanyUuid, resolveApiVersion, resolveBaseUrl } from "./env.ts";
import { ExitCode } from "./exit-codes.ts";
Expand Down Expand Up @@ -54,12 +55,13 @@ export interface ApiContextOpts extends AuthOpts {
* as a follow-up. */
export function buildApiClient(
globals: GlobalFlags,
opts: { baseUrl: string; token: string; stderr?: NodeJS.WritableStream },
opts: { baseUrl: string; token: string; installId?: string; stderr?: NodeJS.WritableStream },
): ApiClient {
return new ApiClient({
baseUrl: opts.baseUrl,
token: opts.token,
apiVersion: resolveApiVersion(),
installId: opts.installId,
observer: globals.verbose ? stderrRequestObserver(opts.stderr ?? process.stderr) : undefined,
});
}
Expand Down Expand Up @@ -131,7 +133,8 @@ export async function resolveApiContext(
const { token, source: tokenSource } = resolved;

const baseUrl = resolveBaseUrl(globals.env);
const client = buildApiClient(globals, { baseUrl, token });
const installId = await resolveInstallIdHeader();
const client = buildApiClient(globals, { baseUrl, token, installId });

if (opts.requireCompany === false) {
return { ok: true, ctx: { client, baseUrl, tokenSource, hasCompany: false } };
Expand Down Expand Up @@ -162,7 +165,7 @@ export async function resolveApiContext(
/** The token from the stored login session, refreshed on near-expiry; null if none. */
async function sessionToken(globals: GlobalFlags, opts: AuthOpts): Promise<string | null> {
const store = opts.store ?? resolveStore();
const http = opts.http ?? oauthHttp(globals);
const http = opts.http ?? (await oauthHttp(globals));
try {
return await getValidUserToken(store, defaultEnv(globals.env), http, opts.now);
} catch (err) {
Expand Down
103 changes: 103 additions & 0 deletions src/lib/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@ import { tmpdir } from "node:os";
import path from "node:path";
import {
type ConfigPaths,
getOrCreateInstallId,
normalizeValue,
readConfig,
resetConfig,
resolveInstallIdHeader,
validateKey,
validateValue,
writeConfig,
Expand Down Expand Up @@ -131,3 +133,104 @@ describe("read/write/reset", () => {
expect(await readConfig(paths)).toEqual({});
});
});

describe("getOrCreateInstallId", () => {
test("generates a v4-shaped UUID on first call and persists it", async () => {
const id = await getOrCreateInstallId(paths);
expect(id).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/);
expect(await readConfig(paths)).toEqual({ install_id: id });
});

test("second call returns the same value without rewriting", async () => {
const first = await getOrCreateInstallId(paths);
const second = await getOrCreateInstallId(paths);
expect(second).toBe(first);
});

test("preserves other config keys when generating", async () => {
await writeConfig({ environment: "sandbox", format: "human" }, paths);
const id = await getOrCreateInstallId(paths);
expect(await readConfig(paths)).toEqual({ environment: "sandbox", format: "human", install_id: id });
});

test("regenerates after resetConfig", async () => {
const first = await getOrCreateInstallId(paths);
await resetConfig(paths);
const second = await getOrCreateInstallId(paths);
expect(second).not.toBe(first);
});

test("readConfig drops an empty-string install_id from disk", async () => {
await Bun.write(paths.file, `install_id = ""\n`);
expect(await readConfig(paths)).toEqual({});
});

test("readConfig drops a non-UUID install_id from disk", async () => {
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 });
});


test("corrupted install_id is regenerated on next getOrCreateInstallId", async () => {
await Bun.write(paths.file, `install_id = "corrupted-value"\n`);
const fresh = await getOrCreateInstallId(paths);
expect(fresh).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/);
expect((await readConfig(paths)).install_id).toBe(fresh);
});

test("concurrent first-run calls each produce a valid install_id and the file converges to one", async () => {
// Documented behavior: two racing callers can each generate + write their own UUID. The
// file ends up with whichever wrote last, and any future caller sees that value. Formal
// first-writer-wins would require a lock — accepted trade-off, since divergence is bounded
// to one command per racing caller and self-heals on the next call.
const [a, b] = await Promise.all([getOrCreateInstallId(paths), getOrCreateInstallId(paths)]);
const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/;
expect(a).toMatch(uuidPattern);
expect(b).toMatch(uuidPattern);
const settled = await getOrCreateInstallId(paths);
expect([a, b]).toContain(settled);
});

test("writeConfig cleans up its temp file on rename failure", async () => {
// A file at paths.dir makes mkdir fail with ENOTDIR AND rename fail. Verify writeConfig
// throws (propagated to caller) and doesn't leave a .tmp behind under the parent scratch dir.
const badDir = path.join(scratch, "not-a-dir");
await Bun.write(badDir, "");
const broken: ConfigPaths = { dir: badDir, file: path.join(badDir, "config.toml") };
await expect(writeConfig({ install_id: "x" }, broken)).rejects.toThrow();
const { readdirSync } = await import("node:fs");
// No `.tmp` sibling under scratch either — the write couldn't proceed past mkdir.
for (const entry of readdirSync(scratch)) {
expect(entry.endsWith(".tmp")).toBe(false);
}
});
});

describe("resolveInstallIdHeader", () => {
test("returns a UUID when telemetry is enabled and the config path is writable", async () => {
const id = await resolveInstallIdHeader(paths);
expect(id).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/);
});

test("returns undefined without side-effects when GUSTO_TELEMETRY=0", async () => {
const prev = process.env.GUSTO_TELEMETRY;
process.env.GUSTO_TELEMETRY = "0";
try {
expect(await resolveInstallIdHeader(paths)).toBeUndefined();
// No config file should have been written under opt-out.
expect(await readConfig(paths)).toEqual({});
} finally {
if (prev === undefined) delete process.env.GUSTO_TELEMETRY;
else process.env.GUSTO_TELEMETRY = prev;
}
});

test("returns undefined (fail-open) when the config path is unwritable", async () => {
// A regular file where the config dir should be: mkdir hits ENOTDIR, getOrCreateInstallId
// throws, resolveInstallIdHeader must catch and silently degrade telemetry.
const badDir = path.join(scratch, "config-blocked");
await Bun.write(badDir, "");
const broken: ConfigPaths = { dir: badDir, file: path.join(badDir, "config.toml") };
expect(await resolveInstallIdHeader(broken)).toBeUndefined();
});
});
68 changes: 65 additions & 3 deletions src/lib/config.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { randomUUID } from "node:crypto";
import { homedir } from "node:os";
import path from "node:path";
import { parse, stringify } from "smol-toml";
import { isTelemetryEnabled } from "./env.ts";
import type { Environment } from "./global-flags.ts";
import type { OutputMode } from "./output.ts";

Expand All @@ -14,12 +16,18 @@ export interface UserConfig {
environment?: Environment;
format?: OutputMode;
skills_auto_install?: SkillsAutoInstall;
/** Anonymous per-install UUID managed by getOrCreateInstallId; not user-configurable. */
install_id?: string;
}

const ENV_VALUES: readonly Environment[] = ["sandbox", "production"] as const;
const FORMAT_VALUES: readonly OutputMode[] = ["agent", "human"] as const;
const SKILLS_AUTO_INSTALL_VALUES: readonly SkillsAutoInstall[] = ["ask", "always", "never"] as const;

// Permissive UUID shape check — variant intentionally not pinned; we only care that on-disk
// values look like real UUIDs so corruption is rejected.
const INSTALL_ID_PATTERN = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;

// `json` is the advertised alias for `agent` (see the `--json` / `--agent` global flags).
// Accept it as a `format` value and persist it as `agent` so the config mirrors the flags.
const FORMAT_ALIASES: Readonly<Record<string, OutputMode>> = { json: "agent" } as const;
Expand Down Expand Up @@ -55,17 +63,67 @@ export async function readConfig(paths: ConfigPaths = configPaths()): Promise<Us
}

export async function writeConfig(cfg: UserConfig, paths: ConfigPaths = configPaths()): Promise<void> {
const { mkdir, chmod } = await import("node:fs/promises");
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);
Comment on lines +66 to +76

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);

} catch (err) {
// Best-effort tmp cleanup; don't shadow the real error.
await rm(tmp, { force: true }).catch(() => {});
throw err;
}
}

export async function resetConfig(paths: ConfigPaths = configPaths()): Promise<void> {
const { rm } = await import("node:fs/promises");
await rm(paths.file, { force: true });
}

/** Return the persisted anonymous install_id, generating and persisting a UUIDv4 on first use.
* On a genuine first-run race two callers may each generate + write their own UUID: last-writer
* wins on disk and every future caller converges. Not full first-writer-wins semantics (that
* would need a lock file), but adequate — divergence is bounded to one command per racing caller
* and self-heals on the next call. */
export async function getOrCreateInstallId(paths: ConfigPaths = configPaths()): Promise<string> {
const cfg = await readConfig(paths);
if (cfg.install_id) return cfg.install_id;
const install_id = randomUUID();
await writeConfig({ ...cfg, install_id }, paths);
const settled = await readConfig(paths);
return settled.install_id ?? install_id;
}

/** The install_id value to stamp on an outbound request, honoring GUSTO_TELEMETRY opt-out.
* Returns undefined (suppresses the header) when telemetry is disabled or when the on-disk
* config can't be read/written — telemetry is best-effort and must never fail the user's command.
*
* Memoized per process when called with the default paths so the file is read at most once
* per invocation regardless of how many outbound requests the command makes. Callers that pass
* an explicit `paths` (tests) bypass the cache and resolve fresh each call. */
let cachedDefaultInstallId: Promise<string | undefined> | undefined;

export async function resolveInstallIdHeader(paths?: ConfigPaths): Promise<string | undefined> {
if (paths !== undefined) return resolveInstallIdOnce(paths);
if (cachedDefaultInstallId === undefined) cachedDefaultInstallId = resolveInstallIdOnce(configPaths());
return cachedDefaultInstallId;
}

async function resolveInstallIdOnce(paths: ConfigPaths): Promise<string | undefined> {
if (!isTelemetryEnabled()) return undefined;
try {
return await getOrCreateInstallId(paths);
} catch {
return undefined;
}
}

export function validateKey(key: string): ConfigKey | null {
return (CONFIG_KEYS as readonly string[]).includes(key) ? (key as ConfigKey) : null;
}
Expand Down Expand Up @@ -113,6 +171,10 @@ function pickValid(raw: Record<string, unknown>): UserConfig {
) {
out.skills_auto_install = raw.skills_auto_install as SkillsAutoInstall;
}
// Drop corrupted values so getOrCreateInstallId regenerates on next call.
if (typeof raw.install_id === "string" && INSTALL_ID_PATTERN.test(raw.install_id)) {
out.install_id = raw.install_id;
}
return out;
}

Expand Down
21 changes: 21 additions & 0 deletions src/lib/env.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
defaultEnv,
getAccessToken,
getCompanyUuid,
isTelemetryEnabled,
resolveApiVersion,
resolveBaseUrl,
resolveMcpBaseUrl,
Expand Down Expand Up @@ -147,3 +148,23 @@ describe("getCompanyUuid", () => {
expect(getCompanyUuid(undefined, {})).toBeNull();
});
});

describe("isTelemetryEnabled", () => {
test("defaults to true when GUSTO_TELEMETRY is unset", () => {
expect(isTelemetryEnabled({})).toBe(true);
});
test("stays true on an empty value", () => {
expect(isTelemetryEnabled({ GUSTO_TELEMETRY: "" })).toBe(true);
});
test("returns false for 0 / false / no (case-insensitive)", () => {
expect(isTelemetryEnabled({ GUSTO_TELEMETRY: "0" })).toBe(false);
expect(isTelemetryEnabled({ GUSTO_TELEMETRY: "false" })).toBe(false);
expect(isTelemetryEnabled({ GUSTO_TELEMETRY: "FALSE" })).toBe(false);
expect(isTelemetryEnabled({ GUSTO_TELEMETRY: "no" })).toBe(false);
});
test("stays true for explicitly truthy values", () => {
expect(isTelemetryEnabled({ GUSTO_TELEMETRY: "1" })).toBe(true);
expect(isTelemetryEnabled({ GUSTO_TELEMETRY: "true" })).toBe(true);
expect(isTelemetryEnabled({ GUSTO_TELEMETRY: "yes" })).toBe(true);
});
});
9 changes: 9 additions & 0 deletions src/lib/env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,15 @@ export function isTruthy(value: string | undefined): boolean {
return normalized === "1" || normalized === "true" || normalized === "yes";
}

/** Telemetry is opt-out: enabled unless GUSTO_TELEMETRY is set to a recognized falsy value
* (`0`, `false`, or `no`). Anything else — unset, empty, or a truthy value — leaves it on. */
export function isTelemetryEnabled(source: EnvSource = process.env as EnvSource): boolean {
const value = source.GUSTO_TELEMETRY;
if (!value) return true;
const normalized = value.toLowerCase();
return !(normalized === "0" || normalized === "false" || normalized === "no");
}

export function resolveApiVersion(source: EnvSource = process.env as EnvSource): string {
return source.GUSTO_API_VERSION ?? DEFAULT_API_VERSION;
}
Expand Down
Loading
Loading