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
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,48 @@ describe('session-set-defaults tool', () => {
expect(parsed.sessionDefaults?.simulatorName).toBeUndefined();
});

it('should store env as a Record<string, string> default', async () => {
const envVars = { STAGING_ENABLED: '1', DEBUG: 'true' };
const result = await sessionSetDefaultsLogic({ env: envVars }, createContext());

expect(result.isError).toBe(false);
expect(sessionStore.getAll().env).toEqual(envVars);
});

it('should persist env to config when persist is true', async () => {
const yaml = ['schemaVersion: 1', 'sessionDefaults: {}', ''].join('\n');

const writes: { path: string; content: string }[] = [];
const fs = createMockFileSystemExecutor({
existsSync: (targetPath: string) => targetPath === configPath,
readFile: async (targetPath: string) => {
if (targetPath !== configPath) {
throw new Error(`Unexpected readFile path: ${targetPath}`);
}
return yaml;
},
writeFile: async (targetPath: string, content: string) => {
writes.push({ path: targetPath, content });
},
});

await initConfigStore({ cwd, fs });

const envVars = { API_URL: 'https://staging.example.com' };
const result = await sessionSetDefaultsLogic(
{ env: envVars, persist: true },
createContext(),
);

expect(result.content[0].text).toContain('Persisted defaults to');
expect(writes.length).toBe(1);

const parsed = parseYaml(writes[0].content) as {
sessionDefaults?: Record<string, unknown>;
};
expect(parsed.sessionDefaults?.env).toEqual(envVars);
});

it('should not persist when persist is true but no defaults were provided', async () => {
const result = await sessionSetDefaultsLogic({ persist: true }, createContext());

Expand Down
32 changes: 32 additions & 0 deletions src/utils/__tests__/session-aware-tool-factory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,4 +258,36 @@ describe('createSessionAwareTool', () => {
expect(parsed.simulatorId).toBe('SIM-123');
expect(parsed.simulatorName).toBeUndefined();
});

it('deep-merges env so user-provided env vars are additive with session defaults', async () => {
const envSchema = z.object({
scheme: z.string(),
projectPath: z.string().optional(),
env: z.record(z.string(), z.string()).optional(),
});

const envHandler = createSessionAwareTool<z.infer<typeof envSchema>>({
internalSchema: envSchema,
logicFunction: async (params) => ({
content: [{ type: 'text', text: JSON.stringify(params.env) }],
isError: false,
}),
getExecutor: () => createMockExecutor({ success: true }),
requirements: [{ allOf: ['scheme'] }],
});

sessionStore.setDefaults({
scheme: 'App',
projectPath: '/a.xcodeproj',
env: { API_KEY: 'abc123', VERBOSE: '1' },
});

// User provides additional env var; session default env vars should be preserved
const result = await envHandler({ env: { DEBUG: 'true', VERBOSE: '0' } });
expect(result.isError).toBe(false);

const envContent = result.content[0] as { type: 'text'; text: string };
const parsed = JSON.parse(envContent.text);
expect(parsed).toEqual({ API_KEY: 'abc123', DEBUG: 'true', VERBOSE: '0' });
});
});
5 changes: 5 additions & 0 deletions src/utils/session-defaults-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export const sessionDefaultKeys = [
'preferXcodebuild',
'platform',
'bundleId',
'env',
] as const;

export type SessionDefaultKey = (typeof sessionDefaultKeys)[number];
Expand Down Expand Up @@ -54,4 +55,8 @@ export const sessionDefaultsSchema = z.object({
.string()
.optional()
.describe('Default bundle ID for launch/stop/log tools when working on a single app.'),
env: z
.record(z.string(), z.string())
.optional()
.describe('Default environment variables to pass to launched apps.'),
});
1 change: 1 addition & 0 deletions src/utils/session-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export type SessionDefaults = {
preferXcodebuild?: boolean;
platform?: string;
bundleId?: string;
env?: Record<string, string>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mutable env leaks through session store

Medium Severity

Adding env as Record<string, string> introduces shared-reference state in sessionStore. setDefaults/getAll are shallow, so mutating a previously passed or retrieved env object can silently change stored defaults without calling sessionStore.setDefaults, bypassing revision tracking.

Fix in Cursor Fix in Web

};

class SessionStore {
Expand Down
9 changes: 8 additions & 1 deletion src/utils/typed-tool-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,14 @@ function createSessionAwareHandler<TParams, TContext>(opts: {
}

// Start with session defaults merged with explicit args (args override session)
const merged: Record<string, unknown> = { ...sessionStore.getAll(), ...sanitizedArgs };
const sessionDefaults = sessionStore.getAll();
const merged: Record<string, unknown> = { ...sessionDefaults, ...sanitizedArgs };

// Deep-merge env: combine session-default env vars with user-provided ones
// (user-provided keys take precedence on conflict)
if (sessionDefaults.env && typeof sanitizedArgs.env === 'object' && sanitizedArgs.env) {
merged.env = { ...sessionDefaults.env, ...(sanitizedArgs.env as Record<string, string>) };
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Env validation bypass via deep merge

Medium Severity

The env deep-merge guard treats any object-like value as mergeable, so arrays or other non-record objects in sanitizedArgs.env get spread into merged.env before internalSchema.parse. When session defaults contain env, invalid user input can be silently accepted or transformed instead of failing validation.

Fix in Cursor Fix in Web

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty env cannot override defaults

Medium Severity

When sessionDefaults.env exists, passing env: {} no longer overrides defaults. The deep-merge branch rebuilds merged.env from default values, so explicit empty env input is ignored and tools still receive session-level variables.

Fix in Cursor Fix in Web


// Apply exclusive pair pruning: only when caller provided a concrete (non-null/undefined) value
// for any key in the pair. When activated, drop other keys in the pair coming from session defaults.
Expand Down
Loading