Support undefined/null as unset in setEnvVars

.changeset/fix-undefined-env-vars.md

@@ -0,0 +1,24 @@
+---
+'@cloudflare/sandbox': patch
+---
+
+Handle undefined environment variables as "unset" in setEnvVars
+
+Environment variable APIs now properly handle undefined values:
+
+- String values are exported as before
+- undefined/null values now **unset** the variable (runs `unset VAR`)
+
+This enables idiomatic JavaScript patterns:
+
+```typescript
+await sandbox.setEnvVars({
+  API_KEY: 'new-key',
+  OLD_SECRET: undefined // unsets OLD_SECRET
+});
+```
+
+**Before**: `sandbox.setEnvVars({ KEY: undefined })` threw a runtime error
+**After**: `sandbox.setEnvVars({ KEY: undefined })` runs `unset KEY`
+
+TypeScript types now honestly accept `Record<string, string | undefined>`.

packages/sandbox-container/src/core/types.ts

@@ -227,7 +227,7 @@ export interface SessionData {
   activeProcess: string | null;
   createdAt: Date;
   expiresAt?: Date;
-  env?: Record<string, string>;
+  env?: Record<string, string | undefined>;
   cwd?: string;
 }
 
@@ -274,7 +274,7 @@ export interface ProcessOptions {
   sessionId?: string;
   processId?: string;
   timeoutMs?: number;
-  env?: Record<string, string>;
+  env?: Record<string, string | undefined>;
   cwd?: string;
   encoding?: string;
   autoCleanup?: boolean;

packages/sandbox-container/src/services/session-manager.ts

@@ -1,6 +1,11 @@
 // SessionManager Service - Manages persistent execution sessions
 
-import type { ExecEvent, Logger } from '@repo/shared';
+import {
+  type ExecEvent,
+  type Logger,
+  partitionEnvVars,
+  shellEscape
+} from '@repo/shared';
 import type {
   CommandErrorContext,
   CommandNotFoundContext,
@@ -226,7 +231,7 @@ export class SessionManager {
     command: string,
     cwd?: string,
     timeoutMs?: number,
-    env?: Record<string, string>
+    env?: Record<string, string | undefined>
   ): Promise<ServiceResult<RawExecResult>> {
     const lock = this.getSessionLock(sessionId);
 
@@ -298,7 +303,7 @@ export class SessionManager {
     fn: (
       exec: (
         command: string,
-        options?: { cwd?: string; env?: Record<string, string> }
+        options?: { cwd?: string; env?: Record<string, string | undefined> }
       ) => Promise<RawExecResult>
     ) => Promise<T>,
     cwd?: string
@@ -321,7 +326,7 @@ export class SessionManager {
         // Provide exec function that uses the session directly (already under lock)
         const exec = async (
           command: string,
-          options?: { cwd?: string; env?: Record<string, string> }
+          options?: { cwd?: string; env?: Record<string, string | undefined> }
         ): Promise<RawExecResult> => {
           return session.exec(command, options);
         };
@@ -391,7 +396,7 @@ export class SessionManager {
     sessionId: string,
     command: string,
     onEvent: (event: ExecEvent) => Promise<void>,
-    options: { cwd?: string; env?: Record<string, string> } = {},
+    options: { cwd?: string; env?: Record<string, string | undefined> } = {},
     commandId: string,
     lockOptions: { background?: boolean } = {}
   ): Promise<ServiceResult<{ continueStreaming: Promise<void> }>> {
@@ -428,7 +433,7 @@ export class SessionManager {
     sessionId: string,
     command: string,
     onEvent: (event: ExecEvent) => Promise<void>,
-    options: { cwd?: string; env?: Record<string, string> },
+    options: { cwd?: string; env?: Record<string, string | undefined> },
     commandId: string,
     lock: Mutex
   ): Promise<ServiceResult<{ continueStreaming: Promise<void> }>> {
@@ -508,7 +513,7 @@ export class SessionManager {
     sessionId: string,
     command: string,
     onEvent: (event: ExecEvent) => Promise<void>,
-    options: { cwd?: string; env?: Record<string, string> },
+    options: { cwd?: string; env?: Record<string, string | undefined> },
     commandId: string,
     lock: Mutex
   ): Promise<ServiceResult<{ continueStreaming: Promise<void> }>> {
@@ -695,18 +700,48 @@ export class SessionManager {
 
   /**
    * Set environment variables on a session atomically.
-   * All exports are executed under a single lock acquisition.
+   * All exports/unsets are executed under a single lock acquisition.
+   * - String values are exported
+   * - undefined/null values are unset
    */
   async setEnvVars(
     sessionId: string,
-    envVars: Record<string, string>
+    envVars: Record<string, string | undefined>
   ): Promise<ServiceResult<void>> {
+    const { toSet, toUnset } = partitionEnvVars(envVars);
+
     return this.withSession(sessionId, async (exec) => {
-      for (const [key, value] of Object.entries(envVars)) {
-        // Escape the value for safe bash usage
-        const escapedValue = value.replace(/'/g, "'\\''");
-        const exportCommand = `export ${key}='${escapedValue}'`;
+      // Validate all keys first (POSIX portable character set)
+      const allKeys = [...toUnset, ...Object.keys(toSet)];
+      for (const key of allKeys) {
+        if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(key)) {
+          throw {
+            code: ErrorCode.VALIDATION_FAILED,
+            message: `Invalid environment variable name: ${key}`,
+            details: { key }
+          };
+        }
+      }
+
+      for (const key of toUnset) {
+        const unsetCommand = `unset ${key}`;
+        const result = await exec(unsetCommand);
+
+        if (result.exitCode !== 0) {
+          throw {
+            code: ErrorCode.COMMAND_EXECUTION_ERROR,
+            message: `Failed to unset environment variable '${key}': ${result.stderr}`,
+            details: {
+              command: unsetCommand,
+              exitCode: result.exitCode,
+              stderr: result.stderr
+            } satisfies CommandErrorContext
+          };
+        }
+      }
 
+      for (const [key, value] of Object.entries(toSet)) {
+        const exportCommand = `export ${key}=${shellEscape(value)}`;
         const result = await exec(exportCommand);
 
         if (result.exitCode !== 0) {

packages/sandbox-container/src/session.ts

@@ -123,8 +123,8 @@ export interface SessionOptions {
    */
   cwd?: string;
 
-  /** Environment variables for the session */
-  env?: Record<string, string>;
+  /** Environment variables for the session. Undefined values are skipped. */
+  env?: Record<string, string | undefined>;
 
   /** Legacy isolation flag (ignored - kept for compatibility) */
   isolation?: boolean;
@@ -159,8 +159,8 @@ export interface RawExecResult {
 interface ExecOptions {
   /** Override working directory for this command only */
   cwd?: string;
-  /** Environment variables for this command only (does not persist in session) */
-  env?: Record<string, string>;
+  /** Environment variables for this command only (does not persist in session). Undefined values are skipped. */
+  env?: Record<string, string | undefined>;
 }
 
 /** Command handle for tracking and killing running commands */
@@ -806,7 +806,7 @@ export class Session {
     exitCodeFile: string,
     cwd?: string,
     isBackground = false,
-    env?: Record<string, string>,
+    env?: Record<string, string | undefined>,
     pidPipe?: string
   ): string {
     // Create unique FIFO names to prevent collisions
@@ -1003,7 +1003,7 @@ export class Session {
   }
 
   private buildScopedEnvBlocks(
-    env: Record<string, string> | undefined,
+    env: Record<string, string | undefined> | undefined,
     cmdId: string,
     options: { restore: boolean }
   ): { setup: string; cleanup: string } {
@@ -1018,15 +1018,20 @@ export class Session {
     const cleanupLines: string[] = [];
     const cmdSuffix = sanitizeIdentifier(cmdId);
 
-    Object.entries(env).forEach(([key, value], index) => {
+    let validIndex = 0;
+    Object.entries(env).forEach(([key, value]) => {
+      if (value == null) {
+        return;
+      }
+
       if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(key)) {
         throw new Error(`Invalid environment variable name: ${key}`);
       }
 
       const escapedValue = value.replace(/'/g, "'\\''");
 
       if (options.restore) {
-        const stateSuffix = `${cmdSuffix}_${index}`;
+        const stateSuffix = `${cmdSuffix}_${validIndex}`;
         const hasVar = `__SANDBOX_HAS_${stateSuffix}`;
         const prevVar = `__SANDBOX_PREV_${stateSuffix}`;
 
@@ -1046,6 +1051,8 @@ export class Session {
       } else {
         setupLines.push(`  export ${key}='${escapedValue}'`);
       }
+
+      validIndex++;
     });
 
     return {

packages/sandbox-container/tests/services/git-service.test.ts

@@ -65,7 +65,7 @@ describe('GitService', () => {
         try {
           const mockExec = async (
             cmd: string,
-            options?: { cwd?: string; env?: Record<string, string> }
+            options?: { cwd?: string; env?: Record<string, string | undefined> }
           ) => {
             // Delegate to executeInSession mock for compatibility with existing tests
             // Only pass cwd if it's defined to match test expectations

packages/sandbox-container/tests/services/session-manager-locking.test.ts

@@ -135,6 +135,80 @@ describe('SessionManager Locking', () => {
     });
   });
 
+  describe('setEnvVars key validation', () => {
+    it('should reject invalid environment variable names', async () => {
+      const sessionId = 'env-validation-session';
+      await sessionManager.createSession({ id: sessionId, cwd: testDir });
+
+      const result = await sessionManager.setEnvVars(sessionId, {
+        'INVALID-NAME': 'value'
+      });
+
+      expect(result.success).toBe(false);
+      if (!result.success) {
+        expect(result.error.code).toBe('VALIDATION_FAILED');
+        expect(result.error.message).toContain(
+          'Invalid environment variable name'
+        );
+      }
+    });
+
+    it('should reject env var names with spaces', async () => {
+      const sessionId = 'env-space-session';
+      await sessionManager.createSession({ id: sessionId, cwd: testDir });
+
+      const result = await sessionManager.setEnvVars(sessionId, {
+        'HAS SPACE': 'value'
+      });
+
+      expect(result.success).toBe(false);
+      if (!result.success) {
+        expect(result.error.code).toBe('VALIDATION_FAILED');
+      }
+    });
+
+    it('should reject env var names starting with numbers', async () => {
+      const sessionId = 'env-number-session';
+      await sessionManager.createSession({ id: sessionId, cwd: testDir });
+
+      const result = await sessionManager.setEnvVars(sessionId, {
+        '123VAR': 'value'
+      });
+
+      expect(result.success).toBe(false);
+      if (!result.success) {
+        expect(result.error.code).toBe('VALIDATION_FAILED');
+      }
+    });
+
+    it('should accept valid POSIX environment variable names', async () => {
+      const sessionId = 'env-valid-session';
+      await sessionManager.createSession({ id: sessionId, cwd: testDir });
+
+      const result = await sessionManager.setEnvVars(sessionId, {
+        VALID_NAME: 'value',
+        _UNDERSCORE: 'value2',
+        mixedCase123: 'value3'
+      });
+
+      expect(result.success).toBe(true);
+    });
+
+    it('should validate keys for unset operations too', async () => {
+      const sessionId = 'env-unset-validation';
+      await sessionManager.createSession({ id: sessionId, cwd: testDir });
+
+      const result = await sessionManager.setEnvVars(sessionId, {
+        'INVALID;rm -rf /': undefined
+      });
+
+      expect(result.success).toBe(false);
+      if (!result.success) {
+        expect(result.error.code).toBe('VALIDATION_FAILED');
+      }
+    });
+  });
+
   describe('streaming execution locking', () => {
     it('should hold lock during foreground streaming until complete', async () => {
       const sessionId = 'stream-fg-session';

packages/sandbox/src/clients/command-client.ts

@@ -34,7 +34,7 @@ export class CommandClient extends BaseHttpClient {
     sessionId: string,
     options?: {
       timeoutMs?: number;
-      env?: Record<string, string>;
+      env?: Record<string, string | undefined>;
       cwd?: string;
     }
   ): Promise<ExecuteResponse> {
@@ -90,7 +90,7 @@ export class CommandClient extends BaseHttpClient {
     sessionId: string,
     options?: {
       timeoutMs?: number;
-      env?: Record<string, string>;
+      env?: Record<string, string | undefined>;
       cwd?: string;
     }
   ): Promise<ReadableStream<Uint8Array>> {

packages/sandbox/src/clients/process-client.ts

@@ -37,7 +37,7 @@ export class ProcessClient extends BaseHttpClient {
     options?: {
       processId?: string;
       timeoutMs?: number;
-      env?: Record<string, string>;
+      env?: Record<string, string | undefined>;
       cwd?: string;
       encoding?: string;
       autoCleanup?: boolean;

packages/sandbox/src/clients/utility-client.ts

@@ -29,7 +29,7 @@ export interface VersionResponse extends BaseApiResponse {
  */
 export interface CreateSessionRequest {
   id: string;
-  env?: Record<string, string>;
+  env?: Record<string, string | undefined>;
   cwd?: string;
 }