From 45b433671e686d0ec8ccaf7bdf3fe697ce7ee646 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 10 Nov 2026 17:58:06 -0800 Subject: [PATCH] fix(recipes): show credentials recipe scripts inherit, add opt-in passEnv allowlist (#40757) * fix(recipes): warn which credentials recipe scripts inherit, add opt-in passEnv allowlist Recipe lifecycle scripts run locally with Orca's full environment. Keep that default (provider CLIs and git auth rely on it) but name the credential-like variables in Settings and let a recipe opt into passEnv so only listed variables plus a safe base set reach its scripts. * fix(recipes): fail closed on a malformed passEnv instead of dropping the recipe Dropping the recipe let a stored snapshot or a same-id plugin recipe run with the full environment. Keep it with an empty allowlist and a diagnostic, match base variable names exactly outside Windows, and make the warning copy apply to any recipe source. * fix(recipes): import user-agent-platform from its new app-shell location --- skill-guides/orca-per-workspace-env.md | 11 + src/cli/bundled-skill-guides.ts | 11 + .../runtime-recipe-timeouts.test.ts | 54 +++++ src/main/ipc/ephemeral-vm-recipe-context.ts | 17 +- src/main/ipc/ephemeral-vm-runtime-handlers.ts | 7 + src/preload/api/ephemeral-vm-api.ts | 2 + src/preload/api/ephemeral-vm-bridge.ts | 1 + .../recipes/EphemeralVmRecipeRow.fixture.tsx | 49 +++++ .../recipes/EphemeralVmRecipeRow.tsx | 71 ++++++- .../recipes/EphemeralVmsPane.test.tsx | 38 ++++ .../recipes/EphemeralVmsPane.tsx | 14 ++ src/renderer/src/i18n/locales/en.json | 6 +- src/shared/ephemeral-vm-recipe-env.test.ts | 196 ++++++++++++++++++ src/shared/ephemeral-vm-recipe-env.ts | 125 +++++++++++ src/shared/ephemeral-vm-recipe-process.ts | 11 +- src/shared/ephemeral-vm-recipe-step.ts | 1 + src/shared/ephemeral-vm-recipe-timeouts.ts | 8 +- src/shared/orca-yaml-hook-types.ts | 3 + src/shared/orca-yaml.ts | 16 +- 19 files changed, 625 insertions(+), 16 deletions(-) create mode 100644 src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmRecipeRow.fixture.tsx create mode 100644 src/shared/ephemeral-vm-recipe-env.test.ts create mode 100644 src/shared/ephemeral-vm-recipe-env.ts diff --git a/skill-guides/orca-per-workspace-env.md b/skill-guides/orca-per-workspace-env.md index 6a7dfff336b..2ccc5e7cf9a 100644 --- a/skill-guides/orca-per-workspace-env.md +++ b/skill-guides/orca-per-workspace-env.md @@ -301,6 +301,17 @@ timeoutSeconds: destroy: 120 ``` +Recipe scripts inherit Orca's whole environment by default, including any API keys and tokens in +it. Set `passEnv` to pass only the variables the scripts need, as exact names or a prefix ending in +`*`; `PATH`, `HOME`, `USER`, `SHELL`, `LANG`, `TMPDIR` and similar basics are always kept, and the +`ORCA_*` recipe variables are always set. Recipes that use an SSH agent also need `SSH_AUTH_SOCK`, +and provider CLIs need their own credential or config variables. A malformed `passEnv` is reported +and limits the scripts to the basic variables until it is fixed. + +```yaml +passEnv: [GH_TOKEN, GITHUB_TOKEN, 'CLOUDSDK_*'] +``` + A timed-out `create` may still have started a VM, so make `create` idempotent per `ORCA_VM_INSTANCE_ID` or have it record the resource before slow work starts. diff --git a/src/cli/bundled-skill-guides.ts b/src/cli/bundled-skill-guides.ts index 78555b61ed9..f33c96a13e9 100644 --- a/src/cli/bundled-skill-guides.ts +++ b/src/cli/bundled-skill-guides.ts @@ -2114,6 +2114,17 @@ const ORCA_PER_WORKSPACE_ENV_MARKDOWN = [ " destroy: 120", "```", "", + "Recipe scripts inherit Orca's whole environment by default, including any API keys and tokens in", + "it. Set `passEnv` to pass only the variables the scripts need, as exact names or a prefix ending in", + "`*`; `PATH`, `HOME`, `USER`, `SHELL`, `LANG`, `TMPDIR` and similar basics are always kept, and the", + "`ORCA_*` recipe variables are always set. Recipes that use an SSH agent also need `SSH_AUTH_SOCK`,", + "and provider CLIs need their own credential or config variables. A malformed `passEnv` is reported", + "and limits the scripts to the basic variables until it is fixed.", + "", + "```yaml", + "passEnv: [GH_TOKEN, GITHUB_TOKEN, 'CLOUDSDK_*']", + "```", + "", "A timed-out `create` may still have started a VM, so make `create` idempotent per", "`ORCA_VM_INSTANCE_ID` or have it record the resource before slow work starts.", "", diff --git a/src/main/ephemeral-vm/runtime-recipe-timeouts.test.ts b/src/main/ephemeral-vm/runtime-recipe-timeouts.test.ts index 994ff5d5bb2..b21c2e36034 100644 --- a/src/main/ephemeral-vm/runtime-recipe-timeouts.test.ts +++ b/src/main/ephemeral-vm/runtime-recipe-timeouts.test.ts @@ -108,4 +108,58 @@ describe('recipe timeoutSeconds and persisted runtimes', () => { getRuntimeRecipeContext(store, userDataPath, provisioned.runtime.id).recipe ).not.toHaveProperty('timeoutSeconds') }) + + it('keeps passEnv out of the persisted snapshot and re-applies it for lifecycle steps', async () => { + const userDataPath = makeDir('orca-vm-passenv-user-data-') + const repoPath = makeDir('orca-vm-passenv-repo-') + const startPath = join(repoPath, 'start.js') + const pairingCode = encodePairingOffer({ + v: PAIRING_OFFER_VERSION, + endpoint: 'wss://vm.example.com', + deviceToken: 'token', + publicKeyB64: 'public-key' + }) + writeFileSync( + startPath, + `console.log(JSON.stringify({ schemaVersion: 1, pairingCode: ${JSON.stringify(pairingCode)}, projectRoot: '/workspace/repo' }))\n` + ) + const recipe: OrcaVmRecipe = { + id: 'remote-vm', + name: 'Remote VM', + create: `"${process.execPath}" "${startPath}"`, + destroy: './destroy.sh', + passEnv: ['GH_TOKEN'] + } + + const provisioned = await provisionEphemeralVmRuntime({ + userDataPath, + repoPath, + recipe, + repoId: 'repo-1', + now: 1_000 + }) + + expect(provisioned.ok).toBe(true) + if (!provisioned.ok) { + return + } + expect(provisioned.runtime.recipe).not.toHaveProperty('passEnv') + const onDisk = JSON.parse(readFileSync(getEphemeralVmRuntimeStorePath(userDataPath), 'utf8')) + expect(RollbackEphemeralVmRuntimeStoreSchema.safeParse(onDisk).success).toBe(true) + + hooks.recipes = [recipe] + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: getRuntimeRecipeContext only reads getRepo from the store. + const store = { + getRepo: () => ({ id: 'repo-1', path: repoPath, kind: 'git' }) + } as unknown as Store + expect( + getRuntimeRecipeContext(store, userDataPath, provisioned.runtime.id).recipe + ).toMatchObject({ passEnv: ['GH_TOKEN'] }) + + // A different recipe that later took the same id does not change the environment. + hooks.recipes = [{ ...recipe, create: './other-create.sh', passEnv: ['OTHER'] }] + expect( + getRuntimeRecipeContext(store, userDataPath, provisioned.runtime.id).recipe + ).not.toHaveProperty('passEnv') + }) }) diff --git a/src/main/ipc/ephemeral-vm-recipe-context.ts b/src/main/ipc/ephemeral-vm-recipe-context.ts index 3990f9827bf..1595c87df44 100644 --- a/src/main/ipc/ephemeral-vm-recipe-context.ts +++ b/src/main/ipc/ephemeral-vm-recipe-context.ts @@ -110,11 +110,12 @@ export function getRuntimeRecipeContext( // holds the same recipe, so a different checkout later placed there can't run its scripts. const fallback = runtime.recipe && runtime.repoPath ? runtime.recipe : null if (fallback && runtime.repoPath) { - const sameRecipe = (loadHooks(runtime.repoPath)?.environmentRecipes ?? []).some( + const sameRecipe = (loadHooks(runtime.repoPath)?.environmentRecipes ?? []).find( (entry) => entry.id === fallback.id && entry.create === fallback.create ) if (sameRecipe) { - return { runtime, repoPath: runtime.repoPath, recipe: fallback } + const passEnv = sameRecipe.passEnv ? { passEnv: sameRecipe.passEnv } : {} + return { runtime, repoPath: runtime.repoPath, recipe: { ...fallback, ...passEnv } } } } throw new EphemeralVmFailureError( @@ -128,11 +129,15 @@ export function getRuntimeRecipeContext( // Snapshots never persist timeoutSeconds, so take the repo's current limits (lifecycle commands stay // snapshotted). Only when the repo recipe has the same create command, so a different recipe that // later took this id (e.g. a repo recipe shadowing a plugin one) cannot impose its limits. - const limits = - repoRecipe?.timeoutSeconds && repoRecipe.create === runtime.recipe?.create + // passEnv follows the same rule, so suspend/resume/destroy see the environment create saw. + const sameCreate = repoRecipe !== undefined && repoRecipe.create === runtime.recipe?.create + const fromRepo = { + ...(sameCreate && repoRecipe.timeoutSeconds ? { timeoutSeconds: repoRecipe.timeoutSeconds } - : {} - const recipe = runtime.recipe ? { ...runtime.recipe, ...limits } : repoRecipe + : {}), + ...(sameCreate && repoRecipe.passEnv ? { passEnv: repoRecipe.passEnv } : {}) + } + const recipe = runtime.recipe ? { ...runtime.recipe, ...fromRepo } : repoRecipe if (!recipe) { throw new EphemeralVmFailureError( ephemeralVmFailure('recipeNotFound', { recipeId: runtime.recipeId }) diff --git a/src/main/ipc/ephemeral-vm-runtime-handlers.ts b/src/main/ipc/ephemeral-vm-runtime-handlers.ts index 8a721827c74..f06d1116771 100644 --- a/src/main/ipc/ephemeral-vm-runtime-handlers.ts +++ b/src/main/ipc/ephemeral-vm-runtime-handlers.ts @@ -1,5 +1,6 @@ import { app, BrowserWindow, ipcMain } from 'electron' import type { Store } from '../persistence/loading-store/store' +import { listCredentialLikeEnvNames } from '../../shared/ephemeral-vm-recipe-env' import { listEphemeralVmRuntimes, updateEphemeralVmRuntimeStatus @@ -112,9 +113,15 @@ export function registerEphemeralVmRuntimeHandlers(store: Store): void { ipcMain.removeHandler('ephemeralVm:stopCleanup') ipcMain.removeHandler('ephemeralVm:forgetRuntime') ipcMain.removeHandler('ephemeralVm:getCleanupCommand') + ipcMain.removeHandler('ephemeralVm:listCredentialEnvNames') stopRuntimesChangedBroadcast?.() stopRuntimesChangedBroadcast = onEphemeralVmRuntimesChanged(broadcastEphemeralVmRuntimesChanged) + // Names only: the recipe list warns which credentials scripts inherit, never their values. + ipcMain.handle('ephemeralVm:listCredentialEnvNames', () => + listCredentialLikeEnvNames(process.env) + ) + ipcMain.handle('ephemeralVm:listRuntimes', (): EphemeralVmRuntimeRecord[] => { return listEphemeralVmRuntimes(app.getPath('userData')) }) diff --git a/src/preload/api/ephemeral-vm-api.ts b/src/preload/api/ephemeral-vm-api.ts index f1aca3ee2f6..0c01a8bf0b4 100644 --- a/src/preload/api/ephemeral-vm-api.ts +++ b/src/preload/api/ephemeral-vm-api.ts @@ -29,6 +29,8 @@ export type EphemeralVmApi = { diagnostics: NonNullable }[] > + /** Credential-like variable names (never values) recipe scripts would inherit from Orca. */ + listCredentialEnvNames: () => Promise doctor: (args: { repoId: string; recipeId: string }) => Promise provision: (args: { repoId: string diff --git a/src/preload/api/ephemeral-vm-bridge.ts b/src/preload/api/ephemeral-vm-bridge.ts index 2eb8372bb79..dc1b4ea12c7 100644 --- a/src/preload/api/ephemeral-vm-bridge.ts +++ b/src/preload/api/ephemeral-vm-bridge.ts @@ -9,6 +9,7 @@ import { export const ephemeralVmApi = { listRecipes: (args) => ipcRenderer.invoke('ephemeralVm:listRecipes', args), listRecipeCatalog: () => ipcRenderer.invoke('ephemeralVm:listRecipeCatalog'), + listCredentialEnvNames: () => ipcRenderer.invoke('ephemeralVm:listCredentialEnvNames'), doctor: (args) => ipcRenderer.invoke('ephemeralVm:doctor', args), provision: (args) => ipcRenderer.invoke('ephemeralVm:provision', args), cancelProvision: (args) => ipcRenderer.invoke('ephemeralVm:cancelProvision', args), diff --git a/src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmRecipeRow.fixture.tsx b/src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmRecipeRow.fixture.tsx new file mode 100644 index 00000000000..1eb05fb3bcb --- /dev/null +++ b/src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmRecipeRow.fixture.tsx @@ -0,0 +1,49 @@ +import { defineFixture } from '../../../../component-fixtures/component-fixture' +import { EphemeralVmRecipeRow } from './EphemeralVmRecipeRow' + +const entry = { repoId: 'r', repoName: 'acme/web', repoPath: '/repo', recipes: [], diagnostics: [] } +const names = ['ANTHROPIC_API_KEY', 'AWS_SECRET_ACCESS_KEY', 'GITHUB_TOKEN', 'OPENAI_API_KEY'] + +function Rows() { + return ( +
+ {}} + /> + {}} + /> + {}} + /> +
+ ) +} + +export const rows = defineFixture({ width: 720, render: () => }) diff --git a/src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmRecipeRow.tsx b/src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmRecipeRow.tsx index 532f35ef227..f60674bee16 100644 --- a/src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmRecipeRow.tsx +++ b/src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmRecipeRow.tsx @@ -1,21 +1,28 @@ -import { Play } from 'lucide-react' +import { KeyRound, Play } from 'lucide-react' import type React from 'react' import type { OrcaHooks } from '../../../../../../shared/orca-yaml-hook-types' +import { isRecipeEnvNamePassed } from '../../../../../../shared/ephemeral-vm-recipe-env' import { Button } from '../../../ui/button' import { useTranslate } from '@/i18n/use-translate' +import { isWindowsUserAgent } from '@/app-shell/platform/user-agent-platform' type RecipeCatalogEntry = Awaited< ReturnType >[number] type Recipe = NonNullable[number] +const MAX_LISTED_ENV_NAMES = 6 + export function EphemeralVmRecipeRow({ entry, recipe, + credentialEnvNames, onUse }: { entry: RecipeCatalogEntry recipe: Recipe + /** Credential-like names in Orca's environment; null until loaded or when unavailable. */ + credentialEnvNames: readonly string[] | null onUse: () => void }): React.JSX.Element { const t = useTranslate() @@ -34,6 +41,7 @@ export function EphemeralVmRecipeRow({

{recipe.id} · {recipe.create} · {destroyLabel}

+
) } + +function RecipeEnvExposure({ + recipe, + credentialEnvNames +}: { + recipe: Recipe + credentialEnvNames: readonly string[] | null +}): React.JSX.Element | null { + const t = useTranslate() + if (!credentialEnvNames) { + return null + } + const platform = isWindowsUserAgent() ? 'win32' : 'linux' + const passed = credentialEnvNames.filter((name) => + isRecipeEnvNamePassed(name, recipe.passEnv, platform) + ) + if (passed.length === 0) { + return recipe.passEnv ? ( +

+ {t( + 'auto.components.settings.EphemeralVmRecipeRow.envLimited', + 'Scripts only receive the variables listed in passEnv, plus PATH, HOME and similar basics.' + )} +

+ ) : null + } + const listed = passed.slice(0, MAX_LISTED_ENV_NAMES).join(', ') + const names = + passed.length > MAX_LISTED_ENV_NAMES + ? t( + 'auto.components.settings.EphemeralVmRecipeRow.envNamesMore', + '{{names}} +{{count}} more', + { + names: listed, + count: passed.length - MAX_LISTED_ENV_NAMES + } + ) + : listed + return ( +

+ + + {recipe.passEnv + ? t( + 'auto.components.settings.EphemeralVmRecipeRow.envExposedAllowlist', + 'Scripts run on this computer and receive these credentials from passEnv: {{names}}.', + { names } + ) + : t( + 'auto.components.settings.EphemeralVmRecipeRow.envExposedAll', + 'Scripts run on this computer with your full environment, including {{names}}. Set passEnv on the recipe to pass only what it needs.', + { names } + )} + +

+ ) +} diff --git a/src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmsPane.test.tsx b/src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmsPane.test.tsx index 12d5c61ddb1..76723a6b2f3 100644 --- a/src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmsPane.test.tsx +++ b/src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmsPane.test.tsx @@ -81,6 +81,7 @@ describe('EphemeralVmsPane', () => { ] } ]), + listCredentialEnvNames: vi.fn().mockResolvedValue(['ANTHROPIC_API_KEY', 'GITHUB_TOKEN']), doctor: vi.fn().mockResolvedValue({ recipeId: 'cloud-sandbox', repoPath: '/repo', @@ -165,4 +166,41 @@ describe('EphemeralVmsPane', () => { expect(container.textContent).toContain('Newest Recipe') expect(container.textContent).not.toContain('Stale Recipe') }) + + it('names the credential-like variables a recipe inherits, without values', async () => { + const container = await renderPane() + await vi.waitFor(() => + expect(container.querySelector('[data-testid="recipe-env-exposure"]')?.textContent).toContain( + 'ANTHROPIC_API_KEY, GITHUB_TOKEN' + ) + ) + expect(container.textContent).toContain('Set passEnv on the recipe') + }) + + it('lists only allowlisted credentials when the recipe sets passEnv', async () => { + const listRecipeCatalog = window.api.ephemeralVm.listRecipeCatalog as ReturnType + listRecipeCatalog.mockReset().mockResolvedValue([ + { + repoId: 'repo-1', + repoName: 'Repo', + repoPath: '/repo', + diagnostics: [], + recipes: [ + { + id: 'limited', + name: 'Limited', + create: 'create', + passEnv: ['GITHUB_TOKEN'] + } + ] + } + ]) + const container = await renderPane() + await vi.waitFor(() => + expect(container.querySelector('[data-testid="recipe-env-exposure"]')?.textContent).toContain( + 'from passEnv: GITHUB_TOKEN.' + ) + ) + expect(container.textContent).not.toContain('ANTHROPIC_API_KEY') + }) }) diff --git a/src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmsPane.tsx b/src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmsPane.tsx index 9e670dff931..392607d693c 100644 --- a/src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmsPane.tsx +++ b/src/renderer/src/components/settings/virtual-machines/recipes/EphemeralVmsPane.tsx @@ -37,6 +37,7 @@ export function EphemeralVmsPane(): React.JSX.Element { const [agentLaunching, setAgentLaunching] = useState(false) const activeSkillRuntime = useActiveProjectSkillRuntime() const [catalog, setCatalog] = useState([]) + const [credentialEnvNames, setCredentialEnvNames] = useState(null) const [isLoading, setIsLoading] = useState(true) const [promptCopied, setPromptCopied] = useState(false) const mountedRef = useMountedRef() @@ -93,6 +94,18 @@ export function EphemeralVmsPane(): React.JSX.Element { void refresh() }, [refresh]) + useEffect(() => { + // The warning is advisory; if the names can't be read the list still works without it. + void window.api.ephemeralVm + .listCredentialEnvNames() + .then((names) => { + if (mountedRef.current) { + setCredentialEnvNames(names) + } + }) + .catch(() => undefined) + }, [mountedRef]) + useEffect(() => { if (!window.api.plugins?.onChanged) { return @@ -302,6 +315,7 @@ export function EphemeralVmsPane(): React.JSX.Element { key={`${entry.repoId}:${recipe.id}`} entry={entry} recipe={recipe} + credentialEnvNames={credentialEnvNames} onUse={() => openWorkspaceComposerForRecipe(entry.repoId, recipe.id)} /> ))} diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index ae68952df1f..044cfb94a6c 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -12088,7 +12088,11 @@ "deleteAnywayClicked": "Delete Anyway clicked" }, "EphemeralVmRecipeRow": { - "useInWorkspace": "Use in workspace" + "useInWorkspace": "Use in workspace", + "envLimited": "Scripts only receive the variables listed in passEnv, plus PATH, HOME and similar basics.", + "envNamesMore": "{{names}} +{{count}} more", + "envExposedAllowlist": "Scripts run on this computer and receive these credentials from passEnv: {{names}}.", + "envExposedAll": "Scripts run on this computer with your full environment, including {{names}}. Set passEnv on the recipe to pass only what it needs." }, "EphemeralVmRuntimesSection": { "cleanupFailed": "Cleanup failed", diff --git a/src/shared/ephemeral-vm-recipe-env.test.ts b/src/shared/ephemeral-vm-recipe-env.test.ts new file mode 100644 index 00000000000..2f1262c0002 --- /dev/null +++ b/src/shared/ephemeral-vm-recipe-env.test.ts @@ -0,0 +1,196 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { + buildRecipeInheritedEnv, + isCredentialLikeEnvName, + isRecipeEnvNamePassed, + listCredentialLikeEnvNames, + parseRecipePassEnv +} from './ephemeral-vm-recipe-env' +import { runRecipeCommand } from './ephemeral-vm-recipe-process' +import { parseOrcaYaml } from './orca-yaml' + +const tmpRoots: string[] = [] +afterEach(() => { + vi.unstubAllEnvs() + for (const root of tmpRoots.splice(0)) { + rmSync(root, { recursive: true, force: true }) + } +}) + +describe('isCredentialLikeEnvName', () => { + it.each([ + 'ANTHROPIC_API_KEY', + 'OPENAI_API_KEY', + 'GITHUB_TOKEN', + 'GH_TOKEN', + 'NPM_TOKEN', + 'STRIPE_SECRET', + 'DB_PASSWORD', + 'AWS_ACCESS_KEY_ID', + 'AWS_PROFILE', + 'GOOGLE_APPLICATION_CREDENTIALS', + 'AZURE_CLIENT_SECRET', + 'github_pat', + 'SSH_AUTH_SOCK' + ])('flags %s', (name) => { + expect(isCredentialLikeEnvName(name)).toBe(true) + }) + + it.each([ + 'PATH', + 'HOME', + 'SHELL', + 'LANG', + 'KEYCHAIN_PATH', + 'TERM_PROGRAM', + 'NODE_ENV', + 'TOKENIZERS' + ])('does not flag %s', (name) => { + expect(isCredentialLikeEnvName(name)).toBe(false) + }) +}) + +describe('isRecipeEnvNamePassed', () => { + it('passes everything when passEnv is not set', () => { + expect(isRecipeEnvNamePassed('OPENAI_API_KEY', undefined)).toBe(true) + }) + + it('passes the base set, exact names and trailing-* prefixes', () => { + const passEnv = ['GH_TOKEN', 'CLOUDSDK_*'] + expect(isRecipeEnvNamePassed('PATH', passEnv, 'linux')).toBe(true) + expect(isRecipeEnvNamePassed('LC_MESSAGES', passEnv, 'linux')).toBe(true) + expect(isRecipeEnvNamePassed('GH_TOKEN', passEnv, 'linux')).toBe(true) + expect(isRecipeEnvNamePassed('CLOUDSDK_CORE_PROJECT', passEnv, 'linux')).toBe(true) + expect(isRecipeEnvNamePassed('GITHUB_TOKEN', passEnv, 'linux')).toBe(false) + expect(isRecipeEnvNamePassed('gh_token', passEnv, 'linux')).toBe(false) + expect(isRecipeEnvNamePassed('path', passEnv, 'linux')).toBe(false) + }) + + it('matches names case-insensitively on Windows', () => { + expect(isRecipeEnvNamePassed('Path', [], 'win32')).toBe(true) + expect(isRecipeEnvNamePassed('SystemRoot', [], 'win32')).toBe(true) + expect(isRecipeEnvNamePassed('gh_token', ['GH_TOKEN'], 'win32')).toBe(true) + }) +}) + +describe('buildRecipeInheritedEnv', () => { + const source = { + PATH: '/usr/bin', + HOME: '/home/me', + OPENAI_API_KEY: 'sk-x', + GH_TOKEN: 'gh-x', + AWS_REGION: 'us-east-1' + } + + it('copies the full environment without passEnv', () => { + expect(buildRecipeInheritedEnv(source, undefined, 'linux')).toEqual(source) + }) + + it('keeps only the base set and listed names with passEnv', () => { + expect(buildRecipeInheritedEnv(source, ['GH_TOKEN', 'AWS_*'], 'linux')).toEqual({ + PATH: '/usr/bin', + HOME: '/home/me', + GH_TOKEN: 'gh-x', + AWS_REGION: 'us-east-1' + }) + expect(buildRecipeInheritedEnv(source, [], 'linux')).toEqual({ + PATH: '/usr/bin', + HOME: '/home/me' + }) + }) +}) + +describe('listCredentialLikeEnvNames', () => { + it('returns sorted names with values, never values', () => { + expect( + listCredentialLikeEnvNames({ + PATH: '/usr/bin', + OPENAI_API_KEY: 'sk-x', + EMPTY_TOKEN: '', + ANTHROPIC_API_KEY: 'sk-y' + }) + ).toEqual(['ANTHROPIC_API_KEY', 'OPENAI_API_KEY']) + }) +}) + +describe('parseRecipePassEnv', () => { + it('accepts names and prefixes, dropping duplicates', () => { + expect(parseRecipePassEnv(['GH_TOKEN', ' AWS_* ', 'GH_TOKEN'])).toEqual(['GH_TOKEN', 'AWS_*']) + expect(parseRecipePassEnv([])).toEqual([]) + }) + + it.each([['GH_TOKEN=x'], 'GH_TOKEN', [1], ['A*B'], ['']])('rejects %j', (value) => { + expect(parseRecipePassEnv(value)).toBeNull() + }) +}) + +describe('orca.yaml passEnv', () => { + const recipeYaml = (passEnv: string): string => + [ + 'environmentRecipes:', + ' - id: remote-vm', + ' name: Remote VM', + ' create: ./create.sh', + ...(passEnv ? [` passEnv: ${passEnv}`] : []) + ].join('\n') + + it('parses a passEnv list onto the recipe', () => { + expect(parseOrcaYaml(recipeYaml('[GH_TOKEN, "AWS_*"]'))?.environmentRecipes).toEqual([ + { + id: 'remote-vm', + name: 'Remote VM', + create: './create.sh', + passEnv: ['GH_TOKEN', 'AWS_*'] + } + ]) + }) + + it('leaves passEnv unset when omitted', () => { + expect(parseOrcaYaml(recipeYaml(''))?.environmentRecipes?.[0]).not.toHaveProperty('passEnv') + }) + + it('keeps a recipe with a malformed passEnv but limits it to the base environment', () => { + const hooks = parseOrcaYaml(recipeYaml('GH_TOKEN')) + expect(hooks?.environmentRecipes?.[0]?.passEnv).toEqual([]) + expect(hooks?.environmentRecipeDiagnostics).toEqual([ + expect.objectContaining({ index: 0, field: 'passEnv' }) + ]) + }) +}) + +describe('runRecipeCommand passEnv', () => { + async function runAndReadEnv(passEnv?: string[]): Promise> { + const repoPath = mkdtempSync(join(tmpdir(), 'orca-vm-recipe-env-')) + tmpRoots.push(repoPath) + const scriptPath = join(repoPath, 'print-env.js') + writeFileSync(scriptPath, 'process.stdout.write(JSON.stringify(process.env))\n') + const result = await runRecipeCommand({ + command: `"${process.execPath}" "${scriptPath}"`, + repoPath, + context: { recipeId: 'remote-vm', repoPath }, + mode: 'create', + resultSchemaVersion: 1, + ...(passEnv ? { passEnv } : {}) + }) + expect(result.exitCode).toBe(0) + return JSON.parse(result.stdout) + } + + it('passes the full environment by default and only the allowlist with passEnv', async () => { + vi.stubEnv('ORCA_TEST_RECIPE_API_KEY', 'secret-value') + vi.stubEnv('ORCA_TEST_RECIPE_ALLOWED', 'allowed-value') + + const full = await runAndReadEnv() + expect(full.ORCA_TEST_RECIPE_API_KEY).toBe('secret-value') + + const limited = await runAndReadEnv(['ORCA_TEST_RECIPE_ALLOWED']) + expect(limited.ORCA_TEST_RECIPE_API_KEY).toBeUndefined() + expect(limited.ORCA_TEST_RECIPE_ALLOWED).toBe('allowed-value') + // Orca's own recipe variables are always set. + expect(limited.ORCA_VM_MODE).toBe('create') + expect(limited.ORCA_RECIPE_ID).toBe('remote-vm') + }) +}) diff --git a/src/shared/ephemeral-vm-recipe-env.ts b/src/shared/ephemeral-vm-recipe-env.ts new file mode 100644 index 00000000000..0f4f7bf5ec5 --- /dev/null +++ b/src/shared/ephemeral-vm-recipe-env.ts @@ -0,0 +1,125 @@ +// Recipe scripts run locally and inherit Orca's environment. These helpers let a recipe opt into a +// narrower environment (`passEnv`) and let the UI name the credential-like variables a recipe sees. + +/** Variables every recipe keeps even with `passEnv`, so shells and provider CLIs still start. */ +export const RECIPE_BASE_ENV_NAMES: readonly string[] = [ + 'PATH', + 'HOME', + 'USER', + 'LOGNAME', + 'SHELL', + 'LANG', + 'LANGUAGE', + 'LC_ALL', + 'LC_CTYPE', + 'TERM', + 'TZ', + 'TMPDIR', + 'TEMP', + 'TMP', + 'XDG_CONFIG_HOME', + 'XDG_CACHE_HOME', + 'XDG_DATA_HOME', + 'XDG_RUNTIME_DIR', + // Windows: cmd.exe and most CLIs fail to start without these. + 'SYSTEMROOT', + 'SYSTEMDRIVE', + 'WINDIR', + 'COMSPEC', + 'PATHEXT', + 'USERPROFILE', + 'USERNAME', + 'HOMEDRIVE', + 'HOMEPATH', + 'APPDATA', + 'LOCALAPPDATA', + 'PROGRAMDATA', + 'PROGRAMFILES', + 'PROGRAMFILES(X86)', + 'COMMONPROGRAMFILES', + 'OS', + 'PROCESSOR_ARCHITECTURE', + 'NUMBER_OF_PROCESSORS' +] + +const BASE_ENV_NAME_SET = new Set(RECIPE_BASE_ENV_NAMES) + +const CREDENTIAL_ENV_NAME_PATTERN = + /(^|_)(API_?KEY|KEY|TOKEN|SECRET|PASSWORD|PASSWD|PASS|CREDENTIALS?|AUTH|PAT)(_|$)/ + +const CREDENTIAL_ENV_PREFIXES = ['AWS_', 'AZURE_', 'GOOGLE_', 'GCLOUD_', 'CLOUDSDK_AUTH_'] + +// Windows env names are case-insensitive; everywhere else they are exact. +function normalizeEnvName(name: string, platform: NodeJS.Platform): string { + return platform === 'win32' ? name.toUpperCase() : name +} + +/** Names only; a positive match means "show this name in the exposure warning", never its value. */ +export function isCredentialLikeEnvName(name: string): boolean { + const upper = name.toUpperCase() + return ( + CREDENTIAL_ENV_NAME_PATTERN.test(upper) || + CREDENTIAL_ENV_PREFIXES.some((prefix) => upper.startsWith(prefix)) + ) +} + +/** `passEnv` entries are exact names or a prefix ending in `*` (e.g. `AWS_*`). */ +export function isRecipeEnvNamePassed( + name: string, + passEnv: readonly string[] | undefined, + platform: NodeJS.Platform = process.platform +): boolean { + if (!passEnv) { + return true + } + const target = normalizeEnvName(name, platform) + // On Windows this is case-insensitive so `Path`/`SystemRoot` survive. + if (BASE_ENV_NAME_SET.has(target) || target.startsWith('LC_')) { + return true + } + return passEnv.some((entry) => { + const pattern = normalizeEnvName(entry, platform) + return pattern.endsWith('*') ? target.startsWith(pattern.slice(0, -1)) : target === pattern + }) +} + +/** The inherited part of a recipe script's environment. */ +export function buildRecipeInheritedEnv( + source: NodeJS.ProcessEnv, + passEnv: readonly string[] | undefined, + platform: NodeJS.Platform = process.platform +): NodeJS.ProcessEnv { + if (!passEnv) { + return { ...source } + } + const env: NodeJS.ProcessEnv = {} + for (const [name, value] of Object.entries(source)) { + if (value !== undefined && isRecipeEnvNamePassed(name, passEnv, platform)) { + env[name] = value + } + } + return env +} + +/** Sorted credential-like names with a non-empty value in `source`. Never returns values. */ +export function listCredentialLikeEnvNames(source: NodeJS.ProcessEnv): string[] { + return Object.entries(source) + .filter(([name, value]) => Boolean(value) && isCredentialLikeEnvName(name)) + .map(([name]) => name) + .sort((a, b) => a.localeCompare(b)) +} + +/** Parse an orca.yaml `passEnv` list. Returns null when the value is malformed. */ +export function parseRecipePassEnv(value: unknown): string[] | null { + if (!Array.isArray(value)) { + return null + } + const names: string[] = [] + for (const entry of value) { + if (typeof entry !== 'string' || !/^[A-Za-z_][A-Za-z0-9_()]*\*?$/.test(entry.trim())) { + return null + } + names.push(entry.trim()) + } + return [...new Set(names)] +} diff --git a/src/shared/ephemeral-vm-recipe-process.ts b/src/shared/ephemeral-vm-recipe-process.ts index 21d9d972f87..07b425bbd2f 100644 --- a/src/shared/ephemeral-vm-recipe-process.ts +++ b/src/shared/ephemeral-vm-recipe-process.ts @@ -2,6 +2,7 @@ import { quoteCmdDoubleQuoted, quotePosixShell } from './shell/launch/shell-argu import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process' import { StringDecoder } from 'node:string_decoder' import type { EphemeralVmRecipeContext } from './ephemeral-vm-recipe-runner' +import { buildRecipeInheritedEnv } from './ephemeral-vm-recipe-env' import { admitProcessTreeKill } from '@orca/process-host/process-tree-kill-gate' import { GrowingByteBuffer } from '@orca/process-host/growing-byte-buffer' @@ -33,6 +34,8 @@ export async function runRecipeCommand(args: { resultSchemaVersion: 1 | 2 stdin?: string env?: NodeJS.ProcessEnv + /** When set, only these (plus a safe base set) of Orca's own variables reach the script. */ + passEnv?: readonly string[] maxCaptureBytes?: number signal?: AbortSignal /** Wall-clock limit; on expiry the process tree is stopped like a cancel and the result has timedOut. */ @@ -53,10 +56,11 @@ export async function runRecipeCommand(args: { return new Promise((resolve, reject) => { let child: ChildProcessWithoutNullStreams try { + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: spawn without a stdio option always pipes stdin/stdout/stderr. child = spawnCommand(args.command, { cwd: args.repoPath, detached: process.platform !== 'win32', - env: buildRecipeEnv(args.env, args.mode, args.context, args.resultSchemaVersion), + env: buildRecipeEnv(args, args.mode, args.context, args.resultSchemaVersion), shell: true, windowsHide: true }) as ChildProcessWithoutNullStreams @@ -247,13 +251,14 @@ export function killRecipeProcess(child: ChildProcessWithoutNullStreams, force = } function buildRecipeEnv( - env: NodeJS.ProcessEnv | undefined, + { env, passEnv }: { env?: NodeJS.ProcessEnv; passEnv?: readonly string[] }, mode: 'create' | 'suspend' | 'resume' | 'destroy', context: EphemeralVmRecipeContext, resultSchemaVersion: 1 | 2 ): NodeJS.ProcessEnv { return { - ...process.env, + // Why: without passEnv recipes keep inheriting everything; provider CLIs and git auth rely on it. + ...buildRecipeInheritedEnv(process.env, passEnv), ...env, ORCA_VM_MODE: mode, ORCA_VM_INSTANCE_ID: context.instanceId ?? '', diff --git a/src/shared/ephemeral-vm-recipe-step.ts b/src/shared/ephemeral-vm-recipe-step.ts index d3be4316814..c3674e86616 100644 --- a/src/shared/ephemeral-vm-recipe-step.ts +++ b/src/shared/ephemeral-vm-recipe-step.ts @@ -40,6 +40,7 @@ export function runRecipeStep( resultSchemaVersion: getEphemeralVmRecipeResultSchemaVersion(args.recipe), ...(stdin === undefined ? {} : { stdin }), env: args.env, + ...(args.recipe.passEnv ? { passEnv: args.recipe.passEnv } : {}), maxCaptureBytes: args.maxCaptureBytes, signal: args.signal, onSpawn: args.onSpawn, diff --git a/src/shared/ephemeral-vm-recipe-timeouts.ts b/src/shared/ephemeral-vm-recipe-timeouts.ts index 344c256f1b6..92b6f3c5e34 100644 --- a/src/shared/ephemeral-vm-recipe-timeouts.ts +++ b/src/shared/ephemeral-vm-recipe-timeouts.ts @@ -25,12 +25,12 @@ export function getEphemeralVmRecipeTimeoutSeconds( } /** Runtime records keep a recipe snapshot under a strict schema that current and older builds - * share, so the snapshot never carries timeoutSeconds; lifecycle calls re-read the limit from - * the repo's orca.yaml instead (see getRuntimeRecipeContext). */ + * share, so the snapshot never carries timeoutSeconds or passEnv; lifecycle calls re-read them + * from the repo's orca.yaml instead (see getRuntimeRecipeContext). */ export function toEphemeralVmRuntimeRecipeSnapshot( recipe: OrcaVmRecipe -): Omit { - const { timeoutSeconds: _timeoutSeconds, ...snapshot } = recipe +): Omit { + const { timeoutSeconds: _timeoutSeconds, passEnv: _passEnv, ...snapshot } = recipe return snapshot } diff --git a/src/shared/orca-yaml-hook-types.ts b/src/shared/orca-yaml-hook-types.ts index 6206a270540..03370ecc4b6 100644 --- a/src/shared/orca-yaml-hook-types.ts +++ b/src/shared/orca-yaml-hook-types.ts @@ -40,6 +40,9 @@ export type OrcaVmRecipe = { destroyDisabled?: boolean // Per-mode wall-clock limits; orca.yaml accepts one number for every mode or a per-mode map. timeoutSeconds?: OrcaVmRecipeTimeoutSeconds + // Opt-in allowlist of Orca's environment variables the scripts receive (plus a safe base set). + // Omitted means scripts inherit the full environment. + passEnv?: string[] } export type OrcaVmRecipeLifecycleMode = 'create' | 'suspend' | 'resume' | 'destroy' diff --git a/src/shared/orca-yaml.ts b/src/shared/orca-yaml.ts index 63c854f85b6..73c32f3287a 100644 --- a/src/shared/orca-yaml.ts +++ b/src/shared/orca-yaml.ts @@ -13,6 +13,7 @@ import { MAX_ORCA_YAML_COLLECTION_ENTRIES } from './orca-yaml-file-limit' import { parseEphemeralVmRecipeTimeoutSeconds } from './ephemeral-vm-recipe-timeouts' +import { parseRecipePassEnv } from './ephemeral-vm-recipe-env' function asRecord(value: unknown): Record | null { return value !== null && typeof value === 'object' && !Array.isArray(value) @@ -182,6 +183,18 @@ function normalizeVmRecipes(value: unknown): VmRecipeParseResult { message: `Recipe "${id}" ${timeouts.message} Steps keep no time limit.` }) } + const parsedPassEnv = + record.passEnv === undefined ? undefined : parseRecipePassEnv(record.passEnv) + if (parsedPassEnv === null) { + // Why: a typo must not widen the environment back to everything, and dropping the recipe + // would let a same-id plugin recipe or a stored snapshot run with the full environment. + diagnostics.push({ + index, + field: 'passEnv', + message: `Recipe "${id}" passEnv must be a list of environment variable names (a trailing * matches a prefix). Scripts only receive basic variables until it is fixed.` + }) + } + const passEnv = parsedPassEnv === null ? [] : parsedPassEnv return { id, name, @@ -194,7 +207,8 @@ function normalizeVmRecipes(value: unknown): VmRecipeParseResult { ...(destroyDisabled ? { destroyDisabled: true } : {}), ...(timeouts.ok && timeouts.timeoutSeconds ? { timeoutSeconds: timeouts.timeoutSeconds } - : {}) + : {}), + ...(passEnv ? { passEnv } : {}) } }) .filter((entry): entry is OrcaVmRecipe => entry !== null)