diff --git a/.changeset/fix-undefined-env-vars.md b/.changeset/fix-undefined-env-vars.md new file mode 100644 index 00000000..9dec6b65 --- /dev/null +++ b/.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`. diff --git a/packages/sandbox-container/src/core/types.ts b/packages/sandbox-container/src/core/types.ts index 4fd8a7ae..048cd1c4 100644 --- a/packages/sandbox-container/src/core/types.ts +++ b/packages/sandbox-container/src/core/types.ts @@ -227,7 +227,7 @@ export interface SessionData { activeProcess: string | null; createdAt: Date; expiresAt?: Date; - env?: Record; + env?: Record; cwd?: string; } @@ -274,7 +274,7 @@ export interface ProcessOptions { sessionId?: string; processId?: string; timeoutMs?: number; - env?: Record; + env?: Record; cwd?: string; encoding?: string; autoCleanup?: boolean; diff --git a/packages/sandbox-container/src/services/session-manager.ts b/packages/sandbox-container/src/services/session-manager.ts index 19c8a3a3..f2f87d20 100644 --- a/packages/sandbox-container/src/services/session-manager.ts +++ b/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 + env?: Record ): Promise> { const lock = this.getSessionLock(sessionId); @@ -298,7 +303,7 @@ export class SessionManager { fn: ( exec: ( command: string, - options?: { cwd?: string; env?: Record } + options?: { cwd?: string; env?: Record } ) => Promise ) => Promise, 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 } + options?: { cwd?: string; env?: Record } ): Promise => { return session.exec(command, options); }; @@ -391,7 +396,7 @@ export class SessionManager { sessionId: string, command: string, onEvent: (event: ExecEvent) => Promise, - options: { cwd?: string; env?: Record } = {}, + options: { cwd?: string; env?: Record } = {}, commandId: string, lockOptions: { background?: boolean } = {} ): Promise }>> { @@ -428,7 +433,7 @@ export class SessionManager { sessionId: string, command: string, onEvent: (event: ExecEvent) => Promise, - options: { cwd?: string; env?: Record }, + options: { cwd?: string; env?: Record }, commandId: string, lock: Mutex ): Promise }>> { @@ -508,7 +513,7 @@ export class SessionManager { sessionId: string, command: string, onEvent: (event: ExecEvent) => Promise, - options: { cwd?: string; env?: Record }, + options: { cwd?: string; env?: Record }, commandId: string, lock: Mutex ): Promise }>> { @@ -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 + envVars: Record ): Promise> { + 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) { diff --git a/packages/sandbox-container/src/session.ts b/packages/sandbox-container/src/session.ts index e919c6eb..3961892f 100644 --- a/packages/sandbox-container/src/session.ts +++ b/packages/sandbox-container/src/session.ts @@ -123,8 +123,8 @@ export interface SessionOptions { */ cwd?: string; - /** Environment variables for the session */ - env?: Record; + /** Environment variables for the session. Undefined values are skipped. */ + env?: Record; /** 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; + /** Environment variables for this command only (does not persist in session). Undefined values are skipped. */ + env?: Record; } /** Command handle for tracking and killing running commands */ @@ -806,7 +806,7 @@ export class Session { exitCodeFile: string, cwd?: string, isBackground = false, - env?: Record, + env?: Record, pidPipe?: string ): string { // Create unique FIFO names to prevent collisions @@ -1003,7 +1003,7 @@ export class Session { } private buildScopedEnvBlocks( - env: Record | undefined, + env: Record | undefined, cmdId: string, options: { restore: boolean } ): { setup: string; cleanup: string } { @@ -1018,7 +1018,12 @@ 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}`); } @@ -1026,7 +1031,7 @@ export class Session { 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 { diff --git a/packages/sandbox-container/tests/services/git-service.test.ts b/packages/sandbox-container/tests/services/git-service.test.ts index 5b073dfa..585a4ac7 100644 --- a/packages/sandbox-container/tests/services/git-service.test.ts +++ b/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 } + options?: { cwd?: string; env?: Record } ) => { // Delegate to executeInSession mock for compatibility with existing tests // Only pass cwd if it's defined to match test expectations diff --git a/packages/sandbox-container/tests/services/session-manager-locking.test.ts b/packages/sandbox-container/tests/services/session-manager-locking.test.ts index 0c9c8b14..aa9ec94c 100644 --- a/packages/sandbox-container/tests/services/session-manager-locking.test.ts +++ b/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'; diff --git a/packages/sandbox/src/clients/command-client.ts b/packages/sandbox/src/clients/command-client.ts index c4e88b99..55f35fac 100644 --- a/packages/sandbox/src/clients/command-client.ts +++ b/packages/sandbox/src/clients/command-client.ts @@ -34,7 +34,7 @@ export class CommandClient extends BaseHttpClient { sessionId: string, options?: { timeoutMs?: number; - env?: Record; + env?: Record; cwd?: string; } ): Promise { @@ -90,7 +90,7 @@ export class CommandClient extends BaseHttpClient { sessionId: string, options?: { timeoutMs?: number; - env?: Record; + env?: Record; cwd?: string; } ): Promise> { diff --git a/packages/sandbox/src/clients/process-client.ts b/packages/sandbox/src/clients/process-client.ts index 1c308551..c5ad5d1f 100644 --- a/packages/sandbox/src/clients/process-client.ts +++ b/packages/sandbox/src/clients/process-client.ts @@ -37,7 +37,7 @@ export class ProcessClient extends BaseHttpClient { options?: { processId?: string; timeoutMs?: number; - env?: Record; + env?: Record; cwd?: string; encoding?: string; autoCleanup?: boolean; diff --git a/packages/sandbox/src/clients/utility-client.ts b/packages/sandbox/src/clients/utility-client.ts index a162cabf..698e60a3 100644 --- a/packages/sandbox/src/clients/utility-client.ts +++ b/packages/sandbox/src/clients/utility-client.ts @@ -29,7 +29,7 @@ export interface VersionResponse extends BaseApiResponse { */ export interface CreateSessionRequest { id: string; - env?: Record; + env?: Record; cwd?: string; } diff --git a/packages/sandbox/src/sandbox.ts b/packages/sandbox/src/sandbox.ts index 1b8d5923..420b489d 100644 --- a/packages/sandbox/src/sandbox.ts +++ b/packages/sandbox/src/sandbox.ts @@ -26,8 +26,10 @@ import type { } from '@repo/shared'; import { createLogger, + filterEnvVars, getEnvString, isTerminalStatus, + partitionEnvVars, type SessionDeleteResult, shellEscape, TraceContext @@ -271,17 +273,32 @@ export class Sandbox extends Container implements ISandbox { await this.ctx.storage.put('keepAliveEnabled', keepAlive); } - // RPC method to set environment variables - async setEnvVars(envVars: Record): Promise { - // Update local state for new sessions - this.envVars = { ...this.envVars, ...envVars }; + async setEnvVars(envVars: Record): Promise { + const { toSet, toUnset } = partitionEnvVars(envVars); + + for (const key of toUnset) { + delete this.envVars[key]; + } + this.envVars = { ...this.envVars, ...toSet }; - // If default session already exists, update it directly if (this.defaultSession) { - // Set environment variables by executing export commands in the existing session - for (const [key, value] of Object.entries(envVars)) { - const escapedValue = value.replace(/'/g, "'\\''"); - const exportCommand = `export ${key}='${escapedValue}'`; + for (const key of toUnset) { + const unsetCommand = `unset ${key}`; + + const result = await this.client.commands.execute( + unsetCommand, + this.defaultSession + ); + + if (result.exitCode !== 0) { + throw new Error( + `Failed to unset ${key}: ${result.stderr || 'Unknown error'}` + ); + } + } + + for (const [key, value] of Object.entries(toSet)) { + const exportCommand = `export ${key}=${shellEscape(value)}`; const result = await this.client.commands.execute( exportCommand, @@ -1771,7 +1788,7 @@ export class Sandbox extends Container implements ISandbox { processId: options.processId }), ...(options?.timeout !== undefined && { timeoutMs: options.timeout }), - ...(options?.env !== undefined && { env: options.env }), + ...(options?.env !== undefined && { env: filterEnvVars(options.env) }), ...(options?.cwd !== undefined && { cwd: options.cwd }), ...(options?.encoding !== undefined && { encoding: options.encoding }), ...(options?.autoCleanup !== undefined && { @@ -2327,8 +2344,9 @@ export class Sandbox extends Container implements ISandbox { ...this.envVars, ...(options?.env ?? {}) }; + const filteredEnv = filterEnvVars(mergedEnv); const envPayload = - Object.keys(mergedEnv).length > 0 ? mergedEnv : undefined; + Object.keys(filteredEnv).length > 0 ? filteredEnv : undefined; // Create session in container await this.client.utils.createSession({ @@ -2430,13 +2448,27 @@ export class Sandbox extends Container implements ISandbox { gitCheckout: (repoUrl, options) => this.gitCheckout(repoUrl, { ...options, sessionId }), - // Environment management - needs special handling - setEnvVars: async (envVars: Record) => { + setEnvVars: async (envVars: Record) => { + const { toSet, toUnset } = partitionEnvVars(envVars); + try { - // Set environment variables by executing export commands - for (const [key, value] of Object.entries(envVars)) { - const escapedValue = value.replace(/'/g, "'\\''"); - const exportCommand = `export ${key}='${escapedValue}'`; + for (const key of toUnset) { + const unsetCommand = `unset ${key}`; + + const result = await this.client.commands.execute( + unsetCommand, + sessionId + ); + + if (result.exitCode !== 0) { + throw new Error( + `Failed to unset ${key}: ${result.stderr || 'Unknown error'}` + ); + } + } + + for (const [key, value] of Object.entries(toSet)) { + const exportCommand = `export ${key}=${shellEscape(value)}`; const result = await this.client.commands.execute( exportCommand, diff --git a/packages/shared/src/env.ts b/packages/shared/src/env.ts index d7bdfbda..58dc46a8 100644 --- a/packages/shared/src/env.ts +++ b/packages/shared/src/env.ts @@ -12,3 +12,61 @@ export function getEnvString( const value = env?.[key]; return typeof value === 'string' ? value : undefined; } + +/** + * Filter environment variables object to only include string values. + * Skips undefined, null, and non-string values. + * + * Use this when you only need the defined values (e.g., for per-command env + * where undefined means "don't override"). + * + * @param envVars - Object that may contain undefined values + * @returns Clean object with only string values + */ +export function filterEnvVars( + envVars: Record +): Record { + const filtered: Record = {}; + + for (const [key, value] of Object.entries(envVars)) { + if (value != null && typeof value === 'string') { + filtered[key] = value; + } + } + + return filtered; +} + +/** + * Partition environment variables into values to set and keys to unset. + * + * - String values → toSet (will be exported) + * - undefined/null → toUnset (will be unset) + * + * This enables idiomatic JS patterns where undefined means "remove": + * ```typescript + * await sandbox.setEnvVars({ + * API_KEY: 'new-key', // will be set + * OLD_VAR: undefined, // will be unset + * }); + * ``` + */ +export function partitionEnvVars( + envVars: Record +): { + toSet: Record; + toUnset: string[]; +} { + const toSet: Record = {}; + const toUnset: string[] = []; + + for (const [key, value] of Object.entries(envVars)) { + if (value != null && typeof value === 'string') { + toSet[key] = value; + } else { + toUnset.push(key); + } + } + + return { toSet, toUnset }; +} diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index d29e74e0..92413a6c 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -4,7 +4,7 @@ */ // Export environment utilities -export { getEnvString } from './env.js'; +export { filterEnvVars, getEnvString, partitionEnvVars } from './env.js'; // Export git utilities export { extractRepoName, diff --git a/packages/shared/src/interpreter-types.ts b/packages/shared/src/interpreter-types.ts index 78c8552d..24c10d7f 100644 --- a/packages/shared/src/interpreter-types.ts +++ b/packages/shared/src/interpreter-types.ts @@ -13,9 +13,10 @@ export interface CreateContextOptions { cwd?: string; /** - * Environment variables for the context + * Environment variables for the context. + * Undefined values are skipped (treated as "not configured"). */ - envVars?: Record; + envVars?: Record; /** * Request timeout in milliseconds @@ -65,9 +66,10 @@ export interface RunCodeOptions { language?: 'python' | 'javascript' | 'typescript'; /** - * Environment variables for this execution + * Environment variables for this execution. + * Undefined values are skipped (treated as "not configured"). */ - envVars?: Record; + envVars?: Record; /** * Execution timeout in milliseconds diff --git a/packages/shared/src/request-types.ts b/packages/shared/src/request-types.ts index c9298017..c85cc325 100644 --- a/packages/shared/src/request-types.ts +++ b/packages/shared/src/request-types.ts @@ -11,7 +11,7 @@ export interface ExecuteRequest { sessionId?: string; background?: boolean; timeoutMs?: number; - env?: Record; + env?: Record; cwd?: string; } @@ -24,7 +24,7 @@ export interface StartProcessRequest { sessionId?: string; processId?: string; timeoutMs?: number; - env?: Record; + env?: Record; cwd?: string; encoding?: string; autoCleanup?: boolean; @@ -130,7 +130,7 @@ export interface ListFilesRequest { export interface SessionCreateRequest { id?: string; name?: string; - env?: Record; + env?: Record; cwd?: string; } diff --git a/packages/shared/src/types.ts b/packages/shared/src/types.ts index 742a6417..1a70d9c4 100644 --- a/packages/shared/src/types.ts +++ b/packages/shared/src/types.ts @@ -16,8 +16,9 @@ export interface BaseExecOptions { * Environment variables for this command invocation. * Values temporarily override session-level/container-level env for the * duration of the command but do not persist after it completes. + * Undefined values are skipped (treated as "not configured"). */ - env?: Record; + env?: Record; /** * Working directory for command execution @@ -400,9 +401,10 @@ export interface SessionOptions { name?: string; /** - * Environment variables for this session + * Environment variables for this session. + * Undefined values are skipped (treated as "not configured"). */ - env?: Record; + env?: Record; /** * Working directory @@ -886,7 +888,7 @@ export interface ExecutionSession { ): Promise; // Environment management - setEnvVars(envVars: Record): Promise; + setEnvVars(envVars: Record): Promise; // Code interpreter methods createCodeContext(options?: CreateContextOptions): Promise; @@ -1049,7 +1051,7 @@ export interface ISandbox { ): Promise; // Environment management - setEnvVars(envVars: Record): Promise; + setEnvVars(envVars: Record): Promise; // Bucket mounting operations mountBucket( diff --git a/packages/shared/tests/env.test.ts b/packages/shared/tests/env.test.ts new file mode 100644 index 00000000..ba9a8003 --- /dev/null +++ b/packages/shared/tests/env.test.ts @@ -0,0 +1,131 @@ +import { describe, expect, it } from 'vitest'; +import { filterEnvVars, getEnvString, partitionEnvVars } from '../src/env'; + +describe('filterEnvVars', () => { + it('passes through valid string values', () => { + const input = { KEY: 'value', ANOTHER: 'test' }; + expect(filterEnvVars(input)).toEqual(input); + }); + + it('skips undefined values', () => { + const input = { KEY: 'value', MISSING: undefined }; + expect(filterEnvVars(input)).toEqual({ KEY: 'value' }); + }); + + it('skips null values', () => { + const input = { KEY: 'value', MISSING: null } as Record< + string, + string | null + >; + expect(filterEnvVars(input)).toEqual({ KEY: 'value' }); + }); + + it('returns empty object when all values are undefined', () => { + const input = { A: undefined, B: undefined }; + expect(filterEnvVars(input)).toEqual({}); + }); + + it('accepts empty string values', () => { + const input = { EMPTY: '' }; + expect(filterEnvVars(input)).toEqual({ EMPTY: '' }); + }); + + it('handles process.env spreading pattern', () => { + const input = { + PATH: '/usr/bin', + UNSET_VAR: undefined, + HOME: '/home/user' + }; + expect(filterEnvVars(input)).toEqual({ + PATH: '/usr/bin', + HOME: '/home/user' + }); + }); + + it('handles empty input object', () => { + expect(filterEnvVars({})).toEqual({}); + }); + + it('preserves special characters in values', () => { + const input = { SPECIAL: "value with 'quotes' and $pecial chars" }; + expect(filterEnvVars(input)).toEqual(input); + }); +}); + +describe('getEnvString', () => { + it('returns string values', () => { + const env = { KEY: 'value' }; + expect(getEnvString(env, 'KEY')).toBe('value'); + }); + + it('returns undefined for missing keys', () => { + const env = { KEY: 'value' }; + expect(getEnvString(env, 'MISSING')).toBeUndefined(); + }); + + it('returns undefined for non-string values', () => { + const env = { NUM: 123, BOOL: true } as Record; + expect(getEnvString(env, 'NUM')).toBeUndefined(); + expect(getEnvString(env, 'BOOL')).toBeUndefined(); + }); +}); + +describe('partitionEnvVars', () => { + it('partitions string values to toSet', () => { + const input = { KEY: 'value', ANOTHER: 'test' }; + const result = partitionEnvVars(input); + expect(result.toSet).toEqual({ KEY: 'value', ANOTHER: 'test' }); + expect(result.toUnset).toEqual([]); + }); + + it('partitions undefined values to toUnset', () => { + const input = { KEY: 'value', REMOVE: undefined }; + const result = partitionEnvVars(input); + expect(result.toSet).toEqual({ KEY: 'value' }); + expect(result.toUnset).toEqual(['REMOVE']); + }); + + it('partitions null values to toUnset', () => { + const input = { KEY: 'value', REMOVE: null } as Record< + string, + string | null + >; + const result = partitionEnvVars(input); + expect(result.toSet).toEqual({ KEY: 'value' }); + expect(result.toUnset).toEqual(['REMOVE']); + }); + + it('handles mixed set and unset values', () => { + const input = { + KEEP: 'keep-value', + REMOVE1: undefined, + ALSO_KEEP: 'another', + REMOVE2: null + } as Record; + const result = partitionEnvVars(input); + expect(result.toSet).toEqual({ KEEP: 'keep-value', ALSO_KEEP: 'another' }); + expect(result.toUnset).toContain('REMOVE1'); + expect(result.toUnset).toContain('REMOVE2'); + expect(result.toUnset).toHaveLength(2); + }); + + it('handles all undefined input', () => { + const input = { A: undefined, B: undefined }; + const result = partitionEnvVars(input); + expect(result.toSet).toEqual({}); + expect(result.toUnset).toEqual(['A', 'B']); + }); + + it('handles empty input', () => { + const result = partitionEnvVars({}); + expect(result.toSet).toEqual({}); + expect(result.toUnset).toEqual([]); + }); + + it('preserves empty string in toSet', () => { + const input = { EMPTY: '', REMOVE: undefined }; + const result = partitionEnvVars(input); + expect(result.toSet).toEqual({ EMPTY: '' }); + expect(result.toUnset).toEqual(['REMOVE']); + }); +}); diff --git a/tests/e2e/environment-workflow.test.ts b/tests/e2e/environment-workflow.test.ts index fc35bdb7..a9f258e7 100644 --- a/tests/e2e/environment-workflow.test.ts +++ b/tests/e2e/environment-workflow.test.ts @@ -237,4 +237,109 @@ describe('Environment Variables', () => { const grepData = (await grepResponse.json()) as ExecResult; expect(grepData.success).toBe(true); }, 90000); + + test('should handle null as unset in setEnvVars', async () => { + const setResponse = await fetch(`${workerUrl}/api/env/set`, { + method: 'POST', + headers, + body: JSON.stringify({ + envVars: { + UNSET_DEFINED: 'test-value', + UNSET_NULL: null, + UNSET_EMPTY: '' + } + }) + }); + + expect(setResponse.status).toBe(200); + + const definedResponse = await fetch(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ command: 'echo $UNSET_DEFINED' }) + }); + expect(definedResponse.status).toBe(200); + const definedData = (await definedResponse.json()) as ExecResult; + expect(definedData.stdout.trim()).toBe('test-value'); + + const nullResponse = await fetch(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ + command: 'printenv UNSET_NULL || echo "not set"' + }) + }); + expect(nullResponse.status).toBe(200); + const nullData = (await nullResponse.json()) as ExecResult; + expect(nullData.stdout.trim()).toBe('not set'); + + const emptyResponse = await fetch(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ command: 'echo "[$UNSET_EMPTY]"' }) + }); + expect(emptyResponse.status).toBe(200); + const emptyData = (await emptyResponse.json()) as ExecResult; + expect(emptyData.stdout.trim()).toBe('[]'); + }, 30000); + + test('should unset previously set env var when passed null', async () => { + const setResponse = await fetch(`${workerUrl}/api/env/set`, { + method: 'POST', + headers, + body: JSON.stringify({ envVars: { TO_REMOVE: 'initial-value' } }) + }); + expect(setResponse.status).toBe(200); + + const beforeResponse = await fetch(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ command: 'echo $TO_REMOVE' }) + }); + expect(beforeResponse.status).toBe(200); + const beforeData = (await beforeResponse.json()) as ExecResult; + expect(beforeData.stdout.trim()).toBe('initial-value'); + + const unsetResponse = await fetch(`${workerUrl}/api/env/set`, { + method: 'POST', + headers, + body: JSON.stringify({ envVars: { TO_REMOVE: null } }) + }); + expect(unsetResponse.status).toBe(200); + + const afterResponse = await fetch(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ command: 'printenv TO_REMOVE || echo "not set"' }) + }); + expect(afterResponse.status).toBe(200); + const afterData = (await afterResponse.json()) as ExecResult; + expect(afterData.stdout.trim()).toBe('not set'); + }, 30000); + + test('should filter null env vars in per-command execution', async () => { + const validResponse = await fetch(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ + command: 'echo $CMD_VALID', + env: { CMD_VALID: 'valid-value', CMD_INVALID: null } + }) + }); + expect(validResponse.status).toBe(200); + const validData = (await validResponse.json()) as ExecResult; + expect(validData.stdout.trim()).toBe('valid-value'); + + const invalidResponse = await fetch(`${workerUrl}/api/execute`, { + method: 'POST', + headers, + body: JSON.stringify({ + command: 'printenv CMD_INVALID || echo "not set"', + env: { CMD_VALID: 'valid-value', CMD_INVALID: null } + }) + }); + expect(invalidResponse.status).toBe(200); + const invalidData = (await invalidResponse.json()) as ExecResult; + expect(invalidData.stdout.trim()).toBe('not set'); + }, 30000); });