[eas-cli] Polish env:pull diff output

CHANGELOG.md

@@ -8,6 +8,8 @@ This is the log of notable changes to EAS CLI and related packages.
 
 ### 🎉 New features
 
+- [eas-cli] Add diff output to `eas env:pull`. ([#4036](https://github.com/expo/eas-cli/pull/4036) by [@khamilowicz](https://github.com/khamilowicz))
+
 - [eas-cli] Support drill-down from event lists in observe:session. ([#3987](https://github.com/expo/eas-cli/pull/3987) by [@douglowder](https://github.com/douglowder))
 
 ### 🐛 Bug fixes

packages/eas-cli/src/commands/env/__tests__/EnvPull.test.ts

@@ -352,8 +352,8 @@ describe(EnvPull, () => {
   });
 
   describe('existing .env.local file handling', () => {
-    it('preserves existing secret values when they exist', async () => {
-      const existingEnvContent = 'SECRET_KEY=existing-secret-value\nOTHER_VAR=other-value';
+    it.each(['existing-secret-value', ''])('preserves an existing secret value %j', async value => {
+      const existingEnvContent = `SECRET_KEY=${value}\nOTHER_VAR=other-value`;
       jest.mocked(fs.exists).mockImplementation(() => Promise.resolve(true));
       jest.mocked(fs.readFile).mockImplementation(() => Promise.resolve(existingEnvContent));
       jest.mocked(confirmAsync).mockResolvedValue(true);
@@ -376,7 +376,7 @@ describe(EnvPull, () => {
       const fileContent = envFileCall![1] as string;
 
       // Should use existing secret value instead of placeholder
-      expect(fileContent).toContain('SECRET_KEY=existing-secret-value');
+      expect(fileContent).toContain(`SECRET_KEY=${value}`);
       expect(fileContent).not.toContain('# SECRET_KEY=***** (secret)');
 
       // Should log that it reused the local value

packages/eas-cli/src/commands/env/pull.ts

@@ -1,5 +1,4 @@
 import { Args, Flags } from '@oclif/core';
-import dotenv from 'dotenv';
 import * as fs from 'fs-extra';
 import path from 'path';
 
@@ -12,6 +11,10 @@ import {
 } from '../../graphql/queries/EnvironmentVariablesQuery';
 import Log from '../../log';
 import { confirmAsync } from '../../prompts';
+import {
+  formatEnvironmentVariableDiffAsync,
+  getEnvironmentVariableNamesFromEnvFile,
+} from '../../utils/environmentVariableDiff';
 import { promptVariableEnvironmentAsync } from '../../utils/prompts';
 
 export default class EnvPull extends EasCommand {
@@ -76,7 +79,8 @@ export default class EnvPull extends EasCommand {
       }
     );
 
-    if (!nonInteractive && (await fs.exists(targetPath))) {
+    const targetExists = await fs.exists(targetPath);
+    if (!nonInteractive && targetExists) {
       const result = await confirmAsync({
         message: `File ${targetPath} already exists. Do you want to overwrite it?`,
       });
@@ -87,9 +91,12 @@ export default class EnvPull extends EasCommand {
     }
 
     let currentEnvLocal: Record<string, string> = {};
+    let existingVariableNames = new Set<string>();
 
-    if (await fs.exists(targetPath)) {
-      currentEnvLocal = dotenv.parse(await fs.readFile(targetPath, 'utf8'));
+    if (targetExists) {
+      const currentEnvFile = await fs.readFile(targetPath, 'utf8');
+      ({ values: currentEnvLocal, variableNames: existingVariableNames } =
+        getEnvironmentVariableNamesFromEnvFile(currentEnvFile));
     }
 
     const filePrefix = `# Environment: ${environment.toLocaleLowerCase()}\n\n`;
@@ -103,14 +110,22 @@ export default class EnvPull extends EasCommand {
       await fs.mkdir(envDir, { recursive: true });
     }
 
+    const diffLog = await formatEnvironmentVariableDiffAsync({
+      environmentVariables,
+      currentEnvValues: currentEnvLocal,
+      existingVariableNames,
+      envDir,
+      targetExists,
+    });
+
     const skippedSecretVariables: string[] = [];
-    const overridenSecretVariables: string[] = [];
+    const overriddenSecretVariables: string[] = [];
 
     const envFileContentLines = await Promise.all(
       environmentVariables.map(async (variable: EnvironmentVariableWithFileContent) => {
         if (variable.visibility === EnvironmentVariableVisibility.Secret) {
-          if (currentEnvLocal[variable.name]) {
-            overridenSecretVariables.push(variable.name);
+          if (Object.hasOwn(currentEnvLocal, variable.name)) {
+            overriddenSecretVariables.push(variable.name);
             return `${variable.name}=${currentEnvLocal[variable.name]}`;
           }
           skippedSecretVariables.push(variable.name);
@@ -131,9 +146,11 @@ export default class EnvPull extends EasCommand {
       `Pulled plain text and sensitive environment variables from "${environment.toLowerCase()}" environment to ${targetPath}.`
     );
 
-    if (overridenSecretVariables.length > 0) {
+    if (overriddenSecretVariables.length > 0) {
       Log.addNewLineIfNone();
-      Log.log(`Reused local values for following secrets: ${overridenSecretVariables.join('\n')}.`);
+      Log.log(
+        `Reused local values for following secrets: ${overriddenSecretVariables.join('\n')}.`
+      );
     }
 
     if (skippedSecretVariables.length > 0) {
@@ -144,5 +161,12 @@ export default class EnvPull extends EasCommand {
         )}.`
       );
     }
+
+    if (diffLog.length > 0) {
+      Log.addNewLineIfNone();
+      for (const line of diffLog) {
+        Log.log(line);
+      }
+    }
   }
 }

packages/eas-cli/src/utils/__tests__/environmentVariableDiff.test.ts

@@ -0,0 +1,194 @@
+import chalk from 'chalk';
+import fs from 'fs-extra';
+import path from 'path';
+
+import {
+  EnvironmentSecretType,
+  EnvironmentVariableScope,
+  EnvironmentVariableVisibility,
+} from '../../graphql/generated';
+import { EnvironmentVariableWithFileContent } from '../../graphql/queries/EnvironmentVariablesQuery';
+import {
+  formatEnvironmentVariableDiffAsync,
+  getEnvironmentVariableNamesFromEnvFile,
+} from '../environmentVariableDiff';
+
+jest.mock('fs-extra');
+
+const envDir = '/project/.eas/.env';
+
+function variable(
+  name: string,
+  value: string | null,
+  overrides: Partial<EnvironmentVariableWithFileContent> = {}
+): EnvironmentVariableWithFileContent {
+  return {
+    id: name,
+    name,
+    value,
+    environments: ['development'],
+    createdAt: new Date().toISOString(),
+    updatedAt: new Date().toISOString(),
+    scope: EnvironmentVariableScope.Project,
+    visibility: EnvironmentVariableVisibility.Public,
+    type: EnvironmentSecretType.String,
+    ...overrides,
+  };
+}
+
+describe(formatEnvironmentVariableDiffAsync, () => {
+  beforeEach(() => {
+    jest.resetAllMocks();
+  });
+
+  it('renders a fresh pull as a neutral baseline', async () => {
+    const result = await formatEnvironmentVariableDiffAsync({
+      environmentVariables: [variable('ONE', '1'), variable('TWO', '2')],
+      currentEnvValues: {},
+      existingVariableNames: new Set(),
+      envDir,
+      targetExists: false,
+    });
+
+    expect(result).toEqual(['  ONE', '  TWO']);
+  });
+
+  it('renders variables as added when the target file exists but is empty', async () => {
+    const result = await formatEnvironmentVariableDiffAsync({
+      environmentVariables: [variable('ONE', '1')],
+      currentEnvValues: {},
+      existingVariableNames: new Set(),
+      envDir,
+      targetExists: true,
+    });
+
+    expect(result).toEqual([chalk.green('+ ONE')]);
+  });
+
+  it('renders added, changed, unchanged, and removed string variables', async () => {
+    const result = await formatEnvironmentVariableDiffAsync({
+      environmentVariables: [
+        variable('ADDED', 'new'),
+        variable('CHANGED', 'new'),
+        variable('UNCHANGED', 'same'),
+      ],
+      currentEnvValues: { CHANGED: 'old', UNCHANGED: 'same', REMOVED: 'old' },
+      existingVariableNames: new Set(['CHANGED', 'UNCHANGED', 'REMOVED']),
+      envDir,
+      targetExists: true,
+    });
+
+    expect(result).toEqual([
+      chalk.green('+ ADDED'),
+      chalk.yellow('~ CHANGED'),
+      '  UNCHANGED',
+      chalk.red('- REMOVED'),
+    ]);
+  });
+
+  it('keeps existing secrets neutral without claiming their values match', async () => {
+    const secret = variable('SECRET', null, {
+      visibility: EnvironmentVariableVisibility.Secret,
+    });
+
+    const result = await formatEnvironmentVariableDiffAsync({
+      environmentVariables: [
+        secret,
+        variable('NEW_SECRET', null, {
+          visibility: EnvironmentVariableVisibility.Secret,
+        }),
+      ],
+      currentEnvValues: {},
+      existingVariableNames: new Set(['SECRET']),
+      envDir,
+      targetExists: true,
+    });
+
+    expect(result).toEqual(['  SECRET', chalk.green('+ NEW_SECRET')]);
+  });
+
+  it('only considers a file unchanged when its output path and contents match', async () => {
+    const fileContents = Buffer.from('contents').toString('base64');
+    const fileVariable = variable('FILE', null, {
+      type: EnvironmentSecretType.FileBase64,
+      valueWithFileContent: fileContents,
+    });
+    const expectedFilePath = path.join(envDir, fileVariable.name);
+    // @ts-expect-error fs-extra overloads are not inferred correctly by Jest.
+    jest.mocked(fs.pathExists).mockResolvedValue(true);
+    // @ts-expect-error fs-extra overloads are not inferred correctly by Jest.
+    jest.mocked(fs.readFile).mockResolvedValue(fileContents);
+
+    await expect(
+      formatEnvironmentVariableDiffAsync({
+        environmentVariables: [fileVariable],
+        currentEnvValues: { FILE: expectedFilePath },
+        existingVariableNames: new Set(['FILE']),
+        envDir,
+        targetExists: true,
+      })
+    ).resolves.toEqual(['  FILE']);
+
+    // @ts-expect-error fs-extra overloads are not inferred correctly by Jest.
+    jest.mocked(fs.readFile).mockResolvedValue(Buffer.from('different').toString('base64'));
+    await expect(
+      formatEnvironmentVariableDiffAsync({
+        environmentVariables: [fileVariable],
+        currentEnvValues: { FILE: expectedFilePath },
+        existingVariableNames: new Set(['FILE']),
+        envDir,
+        targetExists: true,
+      })
+    ).resolves.toEqual([chalk.yellow('~ FILE')]);
+
+    await expect(
+      formatEnvironmentVariableDiffAsync({
+        environmentVariables: [fileVariable],
+        currentEnvValues: { FILE: '/some/other/file' },
+        existingVariableNames: new Set(['FILE']),
+        envDir,
+        targetExists: true,
+      })
+    ).resolves.toEqual([chalk.yellow('~ FILE')]);
+  });
+
+  it('reports missing or unreadable file variables as changed instead of failing', async () => {
+    const fileVariable = variable('FILE', null, {
+      type: EnvironmentSecretType.FileBase64,
+      valueWithFileContent: Buffer.from('contents').toString('base64'),
+    });
+    const expectedFilePath = path.join(envDir, fileVariable.name);
+    const options = {
+      environmentVariables: [fileVariable],
+      currentEnvValues: { FILE: expectedFilePath },
+      existingVariableNames: new Set(['FILE']),
+      envDir,
+      targetExists: true,
+    };
+
+    // @ts-expect-error fs-extra overloads are not inferred correctly by Jest.
+    jest.mocked(fs.pathExists).mockResolvedValue(false);
+    await expect(formatEnvironmentVariableDiffAsync(options)).resolves.toEqual([
+      chalk.yellow('~ FILE'),
+    ]);
+
+    // @ts-expect-error fs-extra overloads are not inferred correctly by Jest.
+    jest.mocked(fs.pathExists).mockResolvedValue(true);
+    // @ts-expect-error fs-extra overloads are not inferred correctly by Jest.
+    jest.mocked(fs.readFile).mockRejectedValue(new Error('EACCES'));
+    await expect(formatEnvironmentVariableDiffAsync(options)).resolves.toEqual([
+      chalk.yellow('~ FILE'),
+    ]);
+  });
+});
+
+describe(getEnvironmentVariableNamesFromEnvFile, () => {
+  it('includes secret placeholders in the existing variable names', () => {
+    const result = getEnvironmentVariableNamesFromEnvFile(
+      ['PLAIN=value', '# SECRET=***** (secret)', '# unrelated comment'].join('\n')
+    );
+
+    expect(result.values).toEqual({ PLAIN: 'value' });
+    expect(result.variableNames).toEqual(new Set(['PLAIN', 'SECRET']));
+  });
+});

packages/eas-cli/src/utils/environmentVariableDiff.ts

@@ -0,0 +1,103 @@
+import chalk from 'chalk';
+import dotenv from 'dotenv';
+import fs from 'fs-extra';
+import path from 'path';
+
+import { EnvironmentSecretType, EnvironmentVariableVisibility } from '../graphql/generated';
+import { EnvironmentVariableWithFileContent } from '../graphql/queries/EnvironmentVariablesQuery';
+
+interface EnvironmentVariableDiffOptions {
+  environmentVariables: EnvironmentVariableWithFileContent[];
+  currentEnvValues: Record<string, string>;
+  existingVariableNames: Set<string>;
+  envDir: string;
+  targetExists: boolean;
+}
+
+export function getEnvironmentVariableNamesFromEnvFile(contents: string): {
+  values: Record<string, string>;
+  variableNames: Set<string>;
+} {
+  const values = dotenv.parse(contents);
+  const variableNames = new Set(Object.keys(values));
+  const secretPlaceholderPattern = /^#\s+([^=\s]+)=\*{5}\s+\(secret\)\s*$/gm;
+
+  for (const match of contents.matchAll(secretPlaceholderPattern)) {
+    variableNames.add(match[1]);
+  }
+
+  return { values, variableNames };
+}
+
+async function isFileVariableUnchangedAsync(
+  currentEnvValue: string | undefined,
+  newVariable: EnvironmentVariableWithFileContent,
+  envDir: string
+): Promise<boolean> {
+  const expectedFilePath = path.join(envDir, newVariable.name);
+  if (currentEnvValue !== expectedFilePath || !newVariable.valueWithFileContent) {
+    return false;
+  }
+
+  try {
+    if (!(await fs.pathExists(currentEnvValue))) {
+      return false;
+    }
+    return (await fs.readFile(currentEnvValue, 'base64')) === newVariable.valueWithFileContent;
+  } catch {
+    // Diff rendering should never prevent env:pull from replacing an unreadable file.
+    return false;
+  }
+}
+
+async function isEnvironmentVariableUnchangedAsync(
+  currentEnvValue: string | undefined,
+  newVariable: EnvironmentVariableWithFileContent,
+  envDir: string
+): Promise<boolean> {
+  if (newVariable.visibility === EnvironmentVariableVisibility.Secret) {
+    // env:pull preserves an existing local secret; it cannot compare it with the remote value.
+    return true;
+  }
+  if (newVariable.type === EnvironmentSecretType.FileBase64) {
+    return await isFileVariableUnchangedAsync(currentEnvValue, newVariable, envDir);
+  }
+  return currentEnvValue === newVariable.value;
+}
+
+export async function formatEnvironmentVariableDiffAsync({
+  environmentVariables,
+  currentEnvValues,
+  existingVariableNames,
+  envDir,
+  targetExists,
+}: EnvironmentVariableDiffOptions): Promise<string[]> {
+  if (!targetExists) {
+    return environmentVariables.map(variable => `  ${variable.name}`);
+  }
+
+  const newVariablesByName = new Map(
+    environmentVariables.map(variable => [variable.name, variable])
+  );
+  const diffLog: string[] = [];
+
+  for (const variable of environmentVariables) {
+    if (!existingVariableNames.has(variable.name)) {
+      diffLog.push(chalk.green(`+ ${variable.name}`));
+    } else if (
+      await isEnvironmentVariableUnchangedAsync(currentEnvValues[variable.name], variable, envDir)
+    ) {
+      diffLog.push(`  ${variable.name}`);
+    } else {
+      diffLog.push(chalk.yellow(`~ ${variable.name}`));
+    }
+  }
+
+  for (const variableName of existingVariableNames) {
+    if (!newVariablesByName.has(variableName)) {
+      diffLog.push(chalk.red(`- ${variableName}`));
+    }
+  }
+
+  return diffLog;
+}