fix: avoid overwriting malformed global config

src/cli/__tests__/global-config.test.ts

@@ -82,9 +82,9 @@ describe('global-config', () => {
     it('creates directory and writes config when none exists', async () => {
       const fresh = createTempConfig('gc-fresh');
 
-      const ok = await updateGlobalConfig({ telemetry: { enabled: false } }, fresh.configDir, fresh.configFile);
+      const result = await updateGlobalConfig({ telemetry: { enabled: false } }, fresh.configDir, fresh.configFile);
 
-      expect(ok).toBe(true);
+      expect(result.success).toBe(true);
       const written = JSON.parse(await readFile(fresh.configFile, 'utf-8'));
       expect(written).toEqual({ telemetry: { enabled: false } });
 
@@ -106,21 +106,33 @@ describe('global-config', () => {
       });
     });
 
-    it('returns false on write failures', async () => {
-      const ok = await updateGlobalConfig(
+    it('does not overwrite malformed config and reports why', async () => {
+      await writeFile(tmp.configFile, '{ invalid json');
+
+      const result = await updateGlobalConfig({ telemetry: { enabled: true } }, tmp.configDir, tmp.configFile);
+
+      expect(result.success).toBe(false);
+      expect(result.success ? '' : result.error.message).toMatch(/malformed/);
+      expect(await readFile(tmp.configFile, 'utf-8')).toBe('{ invalid json');
+    });
+
+    it('returns an error on write failures', async () => {
+      const result = await updateGlobalConfig(
         { telemetry: { enabled: true } },
         tmp.testDir + '/\0invalid',
         tmp.testDir + '/\0invalid/config.json'
       );
 
-      expect(ok).toBe(false);
+      expect(result.success).toBe(false);
     });
   });
 
   describe('getOrCreateInstallationId', () => {
     it('generates installationId on first run and returns created: true', async () => {
       const result = await getOrCreateInstallationId(tmp.configDir, tmp.configFile);
 
+      expect(result.success).toBe(true);
+      if (!result.success) throw result.error;
       expect(result.created).toBe(true);
       expect(result.id).toMatch(/^[0-9a-f-]{36}$/);
       const config = await readGlobalConfig(tmp.configFile);
@@ -132,7 +144,18 @@ describe('global-config', () => {
 
       const result = await getOrCreateInstallationId(tmp.configDir, tmp.configFile);
 
-      expect(result).toEqual({ id: 'existing-id', created: false });
+      expect(result).toEqual({ success: true, id: 'existing-id', created: false });
+    });
+
+    it('returns malformed config errors without generating an id', async () => {
+      await writeFile(tmp.configFile, '{ invalid json');
+
+      const result = await getOrCreateInstallationId(tmp.configDir, tmp.configFile);
+
+      expect(result.success).toBe(false);
+      if (result.success) throw new Error('expected failure');
+      expect(result.error.message).toMatch(/malformed/);
+      expect(await readFile(tmp.configFile, 'utf-8')).toBe('{ invalid json');
     });
   });
 });

src/cli/cli.ts

@@ -215,7 +215,11 @@ export const main = async (argv: string[]) => {
   setupGlobalCleanup();
 
   // Generate installationId on first run and show telemetry notice
-  const { created: isFirstRun } = await getOrCreateInstallationId();
+  const installation = await getOrCreateInstallationId();
+  if (!installation.success) {
+    throw installation.error;
+  }
+  const { created: isFirstRun } = installation;
 
   const program = createProgram();
 

src/cli/commands/telemetry/__tests__/telemetry.test.ts

@@ -1,7 +1,7 @@
 import { readGlobalConfig } from '../../../../lib/schemas/io/global-config';
 import { createTempConfig } from '../../../__tests__/helpers/temp-config';
 import { handleTelemetryDisable, handleTelemetryEnable, handleTelemetryStatus } from '../actions';
-import { chmod, mkdir, rm, writeFile } from 'fs/promises';
+import { writeFile } from 'fs/promises';
 import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
 const tmp = createTempConfig('actions');
@@ -27,15 +27,9 @@ describe('telemetry actions', () => {
     });
 
     it('returns false when config write fails', async () => {
-      await rm(tmp.testDir, { recursive: true, force: true });
-      await mkdir(tmp.testDir, { recursive: true });
-      await chmod(tmp.testDir, 0o444);
-
-      const ok = await handleTelemetryDisable(tmp.configDir, tmp.configFile);
+      const ok = await handleTelemetryDisable(tmp.testDir + '/\0invalid', tmp.testDir + '/\0invalid/config.json');
 
       expect(ok).toBe(false);
-
-      await chmod(tmp.testDir, 0o755);
     });
   });
 

src/cli/commands/telemetry/actions.ts

@@ -10,18 +10,18 @@ export async function handleTelemetryDisable(
   configDir = GLOBAL_CONFIG_DIR,
   configFile = GLOBAL_CONFIG_FILE
 ): Promise<boolean> {
-  const ok = await updateGlobalConfig({ telemetry: { enabled: false } }, configDir, configFile);
-  console.log(ok ? 'Telemetry has been disabled.' : `Warning: could not write config to ${configFile}`);
-  return ok;
+  const result = await updateGlobalConfig({ telemetry: { enabled: false } }, configDir, configFile);
+  console.log(result.success ? 'Telemetry has been disabled.' : `Warning: ${result.error.message}`);
+  return result.success;
 }
 
 export async function handleTelemetryEnable(
   configDir = GLOBAL_CONFIG_DIR,
   configFile = GLOBAL_CONFIG_FILE
 ): Promise<boolean> {
-  const ok = await updateGlobalConfig({ telemetry: { enabled: true } }, configDir, configFile);
-  console.log(ok ? 'Telemetry has been enabled.' : `Warning: could not write config to ${configFile}`);
-  return ok;
+  const result = await updateGlobalConfig({ telemetry: { enabled: true } }, configDir, configFile);
+  console.log(result.success ? 'Telemetry has been enabled.' : `Warning: ${result.error.message}`);
+  return result.success;
 }
 
 export async function handleTelemetryStatus(configFile = GLOBAL_CONFIG_FILE): Promise<void> {

src/cli/commands/telemetry/command.ts

@@ -14,14 +14,18 @@ export function registerTelemetry(program: Command) {
     .command('disable')
     .description('Disable anonymous usage analytics')
     .action(async () => {
-      await handleTelemetryDisable();
+      if (!(await handleTelemetryDisable())) {
+        process.exitCode = 1;
+      }
     });
 
   telemetry
     .command('enable')
     .description('Enable anonymous usage analytics')
     .action(async () => {
-      await handleTelemetryEnable();
+      if (!(await handleTelemetryEnable())) {
+        process.exitCode = 1;
+      }
     });
 
   telemetry

src/cli/telemetry/config.ts

@@ -44,15 +44,19 @@ export async function resolveTelemetryPreference(config?: GlobalConfig): Promise
 
 /**
  * Resolve and validate resource attributes for the current session.
- * Called once at startup — the returned object is reused for every metric in the session.
+ * Called once at startup - the returned object is reused for every metric in the session.
  * Throws if any attribute fails validation (prevents PII leakage).
  */
 export async function resolveResourceAttributes(mode: 'cli' | 'tui'): Promise<ResourceAttributes> {
-  const { id } = await getOrCreateInstallationId();
+  const installation = await getOrCreateInstallationId();
+  if (!installation.success) {
+    throw installation.error;
+  }
+
   return ResourceAttributesSchema.parse({
     'service.name': 'agentcore-cli',
     'service.version': PACKAGE_VERSION,
-    'agentcore-cli.installation_id': id,
+    'agentcore-cli.installation_id': installation.id,
     'agentcore-cli.session_id': randomUUID(),
     'agentcore-cli.mode': mode,
     'os.type': os.type(),

src/lib/schemas/io/global-config.ts

@@ -1,5 +1,7 @@
+import { toError } from '../../errors/types.js';
+import type { Result } from '../../result.js';
 import { readFileSync } from 'fs';
-import { mkdir, readFile, writeFile } from 'fs/promises';
+import { mkdir, readFile, stat, writeFile } from 'fs/promises';
 import { randomUUID } from 'node:crypto';
 import { homedir } from 'os';
 import { join } from 'path';
@@ -46,20 +48,71 @@ export function readGlobalConfigSync(configFile = GLOBAL_CONFIG_FILE): GlobalCon
   }
 }
 
+export type UpdateGlobalConfigResult = Result;
+export type InstallationIdResult = Result<{ id: string; created: boolean }>;
+
 export async function updateGlobalConfig(
   partial: GlobalConfig,
   configDir = GLOBAL_CONFIG_DIR,
   configFile = GLOBAL_CONFIG_FILE
-): Promise<boolean> {
-  try {
-    const existing = await readGlobalConfig(configFile);
-    const merged: GlobalConfig = mergeConfig(existing, partial);
+): Promise<UpdateGlobalConfigResult> {
+  // Read the existing config strictly: a missing file is fine (start fresh), but a
+  // malformed file must not be silently overwritten with merged-in defaults.
+  const existing = await loadConfigForUpdate(configFile);
+  if (!existing.success) {
+    return existing;
+  }
 
+  try {
+    const merged: GlobalConfig = mergeConfig(existing.config, partial);
     await mkdir(configDir, { recursive: true });
     await writeFile(configFile, JSON.stringify(merged, null, 2), 'utf-8');
-    return true;
-  } catch {
-    return false;
+    return { success: true };
+  } catch (error) {
+    return { success: false, error: new Error(`Failed to write config to ${configFile}: ${toError(error).message}`) };
+  }
+}
+
+type LoadConfigResult = { success: true; config: GlobalConfig } | { success: false; error: Error };
+
+/**
+ * Reads the existing global config for an update. Distinguishes a missing file
+ * (treated as an empty config) from a malformed one (read/parse/schema failure),
+ * so the caller can avoid clobbering a config it could not understand.
+ */
+async function loadConfigForUpdate(configFile: string): Promise<LoadConfigResult> {
+  const existingFile = await configFileExists(configFile);
+  if (!existingFile.success) {
+    return existingFile;
+  }
+  if (!existingFile.exists) {
+    return { success: true, config: {} };
+  }
+
+  try {
+    const data = await readFile(configFile, 'utf-8');
+    return { success: true, config: GlobalConfigSchema.parse(JSON.parse(data)) };
+  } catch (error) {
+    const cause = toError(error);
+    return {
+      success: false,
+      error: new Error(`Config at ${configFile} is malformed: ${cause.message}`, { cause }),
+    };
+  }
+}
+
+type ConfigFileExistsResult = { success: true; exists: boolean } | { success: false; error: Error };
+
+async function configFileExists(path: string): Promise<ConfigFileExistsResult> {
+  try {
+    await stat(path);
+    return { success: true, exists: true };
+  } catch (error) {
+    const cause = toError(error);
+    if ((cause as NodeJS.ErrnoException).code === 'ENOENT') {
+      return { success: true, exists: false };
+    }
+    return { success: false, error: new Error(`Could not access config at ${path}: ${cause.message}`, { cause }) };
   }
 }
 
@@ -78,18 +131,27 @@ function mergeConfig(target: GlobalConfig, source: GlobalConfig): GlobalConfig {
  * `created: true` means this is the first run (ID was just generated).
  *
  * Note: concurrent first-run invocations may each generate a different ID;
- * the last write wins. This is acceptable — the ID only needs to be stable
+ * the last write wins. This is acceptable - the ID only needs to be stable
  * after the first successful write, and CLI invocations are typically sequential.
  */
 export async function getOrCreateInstallationId(
   configDir = GLOBAL_CONFIG_DIR,
   configFile = GLOBAL_CONFIG_FILE
-): Promise<{ id: string; created: boolean }> {
-  const config = await readGlobalConfig(configFile);
-  if (config.installationId) {
-    return { id: config.installationId, created: false };
+): Promise<InstallationIdResult> {
+  const existing = await loadConfigForUpdate(configFile);
+  if (!existing.success) {
+    return existing;
+  }
+
+  if (existing.config.installationId) {
+    return { success: true, id: existing.config.installationId, created: false };
   }
+
   const id = randomUUID();
-  await updateGlobalConfig({ installationId: id }, configDir, configFile);
-  return { id, created: true };
+  const updateResult = await updateGlobalConfig({ installationId: id }, configDir, configFile);
+  if (!updateResult.success) {
+    return updateResult;
+  }
+
+  return { success: true, id, created: true };
 }