diff --git a/src/cli/push.ts b/src/cli/push.ts index 13499f2..5d42a38 100644 --- a/src/cli/push.ts +++ b/src/cli/push.ts @@ -4,7 +4,33 @@ import { loadWorkflowFromFolder } from '../CodeLoader.js'; import { createWorkflow, getWorkflow, updateWorkflow } from '../remote/client.js'; import { resolveWorkflowRemote } from '../remote/config.js'; import type { Command } from './utils.js'; -import { confirm, getFlagValue, getPositional, wantsHelp } from './utils.js'; +import { confirm, getFlagValue, getPositional, hasFlag, wantsHelp } from './utils.js'; + +/** + * Heuristic: does this providesEnv value look like a secret that should never + * leave the machine? providesEnv is committed and pushed in clear, so a secret + * placed there (instead of in `env`, which carries names only) would leak. + * Conservative on purpose - known key prefixes plus long high-entropy tokens - + * so static config (URLs, model names like claude-3-5-sonnet-...) is not flagged. + */ +function looksLikeSecret(value: string): boolean { + if (value.includes('${')) return false; // service/placeholder template + if ( + /(sk-[A-Za-z0-9]{16,}|ghp_[A-Za-z0-9]{20,}|gho_[A-Za-z0-9]{20,}|xox[baprs]-[A-Za-z0-9-]{10,}|AKIA[0-9A-Z]{16})/.test( + value, + ) + ) { + return true; + } + if (/^Bearer\s+\S{16,}/i.test(value)) return true; + return ( + value.length >= 32 && + /[a-z]/.test(value) && + /[A-Z]/.test(value) && + /[0-9]/.test(value) && + /^[A-Za-z0-9_\-./+=]+$/.test(value) + ); +} async function pushOne(dir: string, remoteOverrideName: string | undefined): Promise { const wf = loadWorkflowFromFolder(dir); @@ -21,6 +47,25 @@ async function pushOne(dir: string, remoteOverrideName: string | undefined): Pro } const body = wf.toJSON(); + + if (!hasFlag('--force')) { + const flagged: string[] = []; + for (const node of body.nodes) { + for (const [key, value] of Object.entries(node.providesEnv ?? {})) { + if (typeof value === 'string' && looksLikeSecret(value)) flagged.push(`${node.id}.providesEnv.${key}`); + } + } + if (flagged.length > 0) { + console.error( + `Refusing to push "${wf.id}": these providesEnv values look like secrets:\n` + + flagged.map((f) => ` ${f}`).join('\n') + + '\nprovidesEnv is committed and sent to the remote in clear. Put secrets in a node\'s "env" ' + + '(names only, resolved at run time), not providesEnv. Use --force to push anyway.', + ); + process.exit(1); + } + } + let exists = false; try { await getWorkflow(r.remote, wf.id); @@ -70,6 +115,7 @@ Options: --path Workflow folder path (instead of name lookup) --remote Use a specific remote profile --yes, -y Skip confirmation prompt when replacing an existing workflow + --force Push even if a providesEnv value looks like a secret Examples: light push my-workflow diff --git a/src/env.ts b/src/env.ts index 9c42be4..5eebe20 100644 --- a/src/env.ts +++ b/src/env.ts @@ -80,3 +80,21 @@ export function resolveEnvNames(names: readonly string[]): Record process.env[name] === undefined && fileVars[name] === undefined); + return { missing, envFile }; +} diff --git a/src/executor.ts b/src/executor.ts index 7302b59..272c07c 100644 --- a/src/executor.ts +++ b/src/executor.ts @@ -1,4 +1,5 @@ import { randomUUID } from 'node:crypto'; +import { diagnoseEnvNames } from './env.js'; import { CircularDependencyError, WorkflowTimeoutError } from './errors.js'; import { checkCondition, type Link, type Node } from './models/index.js'; import { LightRunClient } from './runner/index.js'; @@ -191,6 +192,35 @@ function shouldFollowLink(link: Link, output: Record, iteration return true; } +/** + * Warn (to stderr) about declared env vars that will not resolve, instead of + * dropping them silently. Covers both nodes and services. No output when every + * declared name resolves. + */ +function warnUnresolvedEnv(workflow: Workflow): void { + const declared: Array<{ label: string; names: string[] }> = []; + for (const node of workflow.nodes.values()) { + if (node.env && node.env.length > 0) declared.push({ label: `node "${node.name}"`, names: node.env }); + } + for (const svc of workflow.services) { + if (svc.env && svc.env.length > 0) declared.push({ label: `service "${svc.id}"`, names: svc.env }); + } + if (declared.length === 0) return; + const lines: string[] = []; + let envFile: string | null = null; + for (const { label, names } of declared) { + const diag = diagnoseEnvNames(names); + envFile = diag.envFile; + if (diag.missing.length > 0) lines.push(` ${label}: ${diag.missing.join(', ')}`); + } + if (lines.length === 0) return; + const where = envFile ? `process.env or ${envFile}` : 'process.env (no .env found)'; + console.warn( + `Warning: these declared env vars did not resolve (looked in ${where}); ` + + `the container will start without them:\n${lines.join('\n')}`, + ); +} + export async function executeWorkflow( workflow: Workflow, initialData: Record = {}, @@ -240,6 +270,8 @@ async function executeWorkflowInner( throw new CircularDependencyError(workflow.id, ['No entry nodes found']); } + warnUnresolvedEnv(workflow); + const abortController = new AbortController(); const signal = abortController.signal; diff --git a/src/remote/config.ts b/src/remote/config.ts index d095ee4..a2500e5 100644 --- a/src/remote/config.ts +++ b/src/remote/config.ts @@ -63,14 +63,20 @@ function restrictToOwner(path: string): void { stdio: 'ignore', }); } catch { - // best-effort: fall back to inherited profile ACLs + console.warn( + `Warning: could not lock down ${path} (icacls failed). It holds your API key ` + + 'and may be readable by other accounts on this machine.', + ); } return; } try { chmodSync(path, 0o600); } catch { - // best-effort + console.warn( + `Warning: could not chmod ${path} to 0600. It holds your API key and may be ` + + 'readable by other users on this machine.', + ); } }