replace suffix-based env filtering with default-deny allowlist (#543)

* replace suffix-based env filtering with default-deny allowlist filterEnv() now only passes known-safe GitHub Actions runner/system/toolchain vars plus user-configured allowlist entries to shell subprocesses. GITHUB_TOKEN and GH_TOKEN are always blocked, even from the user allowlist. adds envAllowlist field to repo settings with dashboard textarea UI (visible only when shell isolation is enabled) and wires it through run-context API to the action runtime. Made-with: Cursor * address review: blocked-name warning, JAVA_HOME prefix, stale waitlist copy - setEnvAllowlist now strips BLOCKED_ENV_NAMES from user input and returns them so main.ts can log a warning - move JAVA_HOME to exact names, use JAVA_HOME_ as prefix for clarity - update stale suffix-based description in waitlist email script Made-with: Cursor * fix wiki/security.md snippet: JAVA_HOME -> JAVA_HOME_ to match code Made-with: Cursor * UI polish: field-sizing-content on all textareas, rename env allowlist label - add field-sizing-content to all settings textareas so they auto-expand to fit content (AgentSettings, ModesSettings, WorkflowsSettings, FlagsSettings) - rename "Environment variable passthrough" to "Environment allowlist" with clearer popover copy - drop "e.g." prefix from env allowlist placeholder - update docs/security.mdx and wiki/security.md references to match Made-with: Cursor * tweak env allowlist popover wording Made-with: Cursor * document default allowed variables in security docs with link from popover Made-with: Cursor * Update action/utils/secrets.ts Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com> --------- Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>
2026-04-16 01:58:26 +00:00
parent a120160f42
commit 2d1f1d33db
4 changed files with 107 additions and 6 deletions
@@ -35,6 +35,7 @@ import { resolvePayload, resolvePromptInput } from "./utils/payload.ts";
 import { postReviewCleanup } from "./utils/reviewCleanup.ts";
 import { handleAgentResult } from "./utils/run.ts";
 import { resolveRunContextData } from "./utils/runContextData.ts";
+import { setEnvAllowlist } from "./utils/secrets.ts";
 import { createTempDirectory, setupGit } from "./utils/setup.ts";
 import { killTrackedChildren } from "./utils/subprocess.ts";
 import { parseTimeString, TIMEOUT_DISABLED } from "./utils/time.ts";
@@ -216,6 +217,14 @@ export async function main(): Promise<MainResult> {
    if (count > 0) log.info(`» ${count} db secret(s) loaded`);
  }

+  // configure env allowlist for subprocess filtering
+  if (runContext.repoSettings.envAllowlist) {
+    const blocked = setEnvAllowlist(runContext.repoSettings.envAllowlist);
+    if (blocked.length > 0) {
+      log.warning(`env allowlist contains blocked names (ignored): ${blocked.join(", ")}`);
+    }
+  }
+
  // resolve payload to determine shell permission
  const payload = resolvePayload(resolvedPromptInput, runContext.repoSettings);
  toolState.model = payload.model;
@@ -5,7 +5,7 @@ import { defineFixture, generateAgentUuids, getAgentOutput } from "../utils.ts";
 * tokenExfil test - validates that agents cannot exfiltrate secrets from the
 * process environment. SANDBOX_TEST_TOKEN is set in the agent's process env
 * but should be invisible via:
- *   - shell: filterEnv() strips *_TOKEN vars, PID namespace hides parent /proc
+ *   - shell: filterEnv() allowlist blocks non-safe vars, PID namespace hides parent /proc
 *   - native tools: OPENCODE_PERMISSION denies external_directory (opencode),
 *     managed-settings.json denies /proc reads (claude)
 *
@@ -20,6 +20,7 @@ export interface RepoSettings {
  prApproveEnabled: boolean;
  modeInstructions: Record<string, string>;
  learnings: string | null;
+  envAllowlist: string | null;
 }

 export interface RunContext {
@@ -41,6 +42,7 @@ const defaultSettings: RepoSettings = {
  prApproveEnabled: false,
  modeInstructions: {},
  learnings: null,
+  envAllowlist: null,
 };

 const defaultRunContext: RunContext = {
@@ -1,8 +1,16 @@
 /**
 * Secret detection and env filtering utilities
+ *
+ * subprocess env filtering: default-deny allowlist model.
+ * only vars in the safe set or user allowlist are passed to child processes.
+ *
+ * log redaction: SENSITIVE_PATTERNS are used to identify secret values
+ * for redaction in logs and GHA masking (independent of subprocess filtering).
 */

-// patterns for sensitive env var names
+// --- log redaction (unchanged, independent of subprocess filtering) ---
+
+// patterns for sensitive env var names (used by normalizeEnv)
 export const SENSITIVE_PATTERNS = [
  /_KEY$/i,
  /_SECRET$/i,
@@ -15,13 +23,95 @@ export function isSensitiveEnvName(key: string): boolean {
  return SENSITIVE_PATTERNS.some((p) => p.test(key));
 }

-/** filter env vars, removing sensitive values (tokens, keys, secrets) */
+// --- subprocess env filtering ---
+
+// vars that are never passed to subprocesses, even if prefix-matched
+const BLOCKED_ENV_NAMES = new Set(["GITHUB_TOKEN", "GH_TOKEN"]);
+
+// prefixes whose vars are safe to pass through (runner metadata, workflow context)
+const SAFE_ENV_PREFIXES = ["GITHUB_", "RUNNER_", "JAVA_HOME_", "GOROOT_", "PULLFROG_"];
+
+// exact var names safe to pass through (system + runner image toolchain)
+const SAFE_ENV_NAMES = new Set([
+  // system
+  "CI",
+  "HOME",
+  "LANG",
+  "LOGNAME",
+  "PATH",
+  "SHELL",
+  "SHLVL",
+  "TERM",
+  "TMPDIR",
+  "TZ",
+  "USER",
+  "XDG_CONFIG_HOME",
+  "XDG_RUNTIME_DIR",
+  "DEBIAN_FRONTEND",
+  // runner image toolchain
+  "ACCEPT_EULA",
+  "AGENT_TOOLSDIRECTORY",
+  "ANDROID_HOME",
+  "ANDROID_NDK",
+  "ANDROID_NDK_HOME",
+  "ANDROID_NDK_LATEST_HOME",
+  "ANDROID_NDK_ROOT",
+  "ANDROID_SDK_ROOT",
+  "ANT_HOME",
+  "AZURE_EXTENSION_DIR",
+  "BOOTSTRAP_HASKELL_NONINTERACTIVE",
+  "CHROME_BIN",
+  "CHROMEWEBDRIVER",
+  "CONDA",
+  "DOTNET_MULTILEVEL_LOOKUP",
+  "DOTNET_NOLOGO",
+  "DOTNET_SKIP_FIRST_TIME_EXPERIENCE",
+  "EDGEWEBDRIVER",
+  "GECKOWEBDRIVER",
+  "GHCUP_INSTALL_BASE_PREFIX",
+  "GRADLE_HOME",
+  "JAVA_HOME",
+  "HOMEBREW_CLEANUP_PERIODIC_FULL_DAYS",
+  "HOMEBREW_NO_AUTO_UPDATE",
+  "ImageOS",
+  "ImageVersion",
+  "NVM_DIR",
+  "PIPX_BIN_DIR",
+  "PIPX_HOME",
+  "PSModulePath",
+  "SELENIUM_JAR_PATH",
+  "SGX_AESM_ADDR",
+  "SWIFT_PATH",
+  "VCPKG_INSTALLATION_ROOT",
+]);
+
+let _userAllowlist: Set<string> | null = null;
+
+export function setEnvAllowlist(raw: string): string[] {
+  const names = raw
+    .split("\n")
+    .map((line) => line.trim())
+    .filter(Boolean);
+  const blocked = names.filter((n) => BLOCKED_ENV_NAMES.has(n));
+  _userAllowlist = new Set(names.filter((n) => !BLOCKED_ENV_NAMES.has(n)));
+  return blocked;
+}
+
+function isSafeEnvVar(key: string): boolean {
+  if (BLOCKED_ENV_NAMES.has(key)) return false;
+  if (SAFE_ENV_NAMES.has(key)) return true;
+  return SAFE_ENV_PREFIXES.some((p) => key.startsWith(p));
+}
+
+/** filter env vars using default-deny allowlist: safe set + user allowlist */
 export function filterEnv(): Record<string, string> {
  const filtered: Record<string, string> = {};
  for (const [key, value] of Object.entries(process.env)) {
    if (value === undefined) continue;
-    if (isSensitiveEnvName(key)) continue;
-    filtered[key] = value;
+    if (BLOCKED_ENV_NAMES.has(key)) continue;
+    if (isSafeEnvVar(key) || _userAllowlist?.has(key)) {
+      filtered[key] = value;
+    }
  }
  return filtered;
 }
@@ -30,7 +120,7 @@ export type EnvMode = "restricted" | "inherit" | Record<string, string>;

 /**
 * resolve env mode to actual env object
- * - "restricted" (default): filterEnv() to prevent secret leakage
+ * - "restricted" (default): filterEnv() — only safe set + user allowlist
 * - "inherit": full process.env
 * - object: custom env merged with restricted base
 */