feat(packagesettings): add ExternalCommands model for env filtering

Add `ExternalCommands` Pydantic model with `keep_env` / `delete_env`
pattern lists, a `filter_env()` method, and a `DEFAULT_KEEP_ENV`
class variable for essential variables (HOME, PATH, LC_*, TERM, TZ,
TMPDIR, etc.).

`delete_env` matching is case-insensitive so credentials cannot slip
through due to unexpected capitalisation.  Non-POSIX env var keys are
always stripped by `filter_env()`.

Not yet wired into `external_commands.run()`.

See: #1083
Co-Authored-By: Claude <claude@anthropic.com>
Signed-off-by: Christian Heimes <cheimes@redhat.com>

Author: tiran

docs/proposals/filter-env.md

@@ -68,20 +68,30 @@ validation error.
 
 ### Evaluation order
 
-`keep_env` is evaluated before `delete_env`. All checks are case-insensitive
-and short-circuit. If a variable matches the hard-coded always-keep set or
-an entry in `keep_env`, then the variable is kept.
+`keep_env` is evaluated before `delete_env`. If a variable matches the
+hard-coded always-keep set or an entry in `keep_env`, then the variable
+is kept.
+
+`delete_env` matching is **case-insensitive** for maximum convenience
+and security -- a pattern `aws_*` removes `AWS_SECRET_ACCESS_KEY`
+regardless of casing, so credentials cannot slip through due to
+unexpected capitalisation. `keep_env` and the always-keep set are
+case-sensitive, matching the exact variable names used in practice.
 
 For each variable in `os.environ`:
 
-1. If it is in a hard-coded always-keep set -- **keep**, regardless of
+1. If the key is not a valid POSIX name (`[A-Za-z_][A-Za-z0-9_]*`) --
+   **delete**. This removes keys with dashes, dots, embedded spaces,
+   or bash-exported function definitions (`BASH_FUNC_*%%`) that no
+   build script should need.
+2. If it is in a hard-coded always-keep set -- **keep**, regardless of
    configuration. The always-keep set contains variables required for
-   basic subprocess operation and proxy settings: `HOME`, `HOSTNAME`,
-   `LANG`, `LANGUAGE`, `LC_*`, `LOGNAME`, `NO_COLOR`, `PATH`, `SHELL`,
-   `USER`, `http_proxy`, `https_proxy`, `no_proxy`.
-2. If any `keep_env` entry matches -- **keep**.
-3. If any `delete_env` entry matches -- **delete**.
-4. Otherwise -- **keep** (default passthrough).
+   basic subprocess operation: `HOME`, `HOSTNAME`, `LANG`, `LANGUAGE`,
+   `LC_*`, `LOGNAME`, `NO_COLOR`, `PATH`, `SHELL`, `TEMP`, `TERM`,
+   `TMP`, `TMPDIR`, `TZ`, `USER`.
+3. If any `keep_env` entry matches (case-sensitive) -- **keep**.
+4. If any `delete_env` entry matches (case-insensitive) -- **delete**.
+5. Otherwise -- **keep** (default passthrough).
 
 `delete_env: ['*']` can be used to prevent passthrough. It filters all
 variables that neither match the always-keep set nor `keep_env` entries.

src/fromager/packagesettings/__init__.py

@@ -4,6 +4,7 @@
 from ._models import (
     BuildOptions,
     DownloadSource,
+    ExternalCommands,
     GitOptions,
     PackageSettings,
     ProjectOverride,
@@ -57,6 +58,7 @@
     "DownloadSource",
     "EnvKey",
     "EnvVars",
+    "ExternalCommands",
     "GitHubTagCloneResolver",
     "GitHubTagDownloadResolver",
     "GitLabTagCloneResolver",

src/fromager/packagesettings/_models.py

@@ -5,14 +5,15 @@
 import logging
 import os
 import pathlib
+import re
 import typing
 from collections.abc import Mapping
 
 import pydantic
 import yaml
 from packaging.requirements import Requirement
 from packaging.utils import canonicalize_name
-from pydantic import AnyUrl, Field
+from pydantic import AnyUrl, Field, PrivateAttr, StringConstraints
 from pydantic_core import core_schema
 
 # from ._resolver import SourceResolver
@@ -72,6 +73,161 @@ class SbomSettings(pydantic.BaseModel):
     """
 
 
+# Environment variable filter patterns for ExternalCommands.
+# Pattern: starts with letter or underscore, rest is letters/digits/underscores,
+# optionally ending with ``*`` (trailing wildcard).
+# DeleteEnvPattern additionally allows bare ``*`` (catch-all).
+KeepEnvPattern = typing.Annotated[
+    str,
+    StringConstraints(pattern=r"^[a-zA-Z_][a-zA-Z0-9_]*\*?$"),
+]
+
+DeleteEnvPattern = typing.Annotated[
+    str,
+    StringConstraints(pattern=r"^(\*|[a-zA-Z_][a-zA-Z0-9_]*\*?)$"),
+]
+
+
+# POSIX.1-2024 sec. 8.1: environment variable names consist of uppercase
+# letters, digits, and underscores and do not begin with a digit.  We
+# also accept lowercase letters for portability (common on Linux).
+_POSIX_ENV_KEY_RE = re.compile(r"[A-Za-z_][A-Za-z0-9_]*")
+
+
+def _compile_env_patterns(
+    patterns: tuple[str, ...],
+    *,
+    case_insensitive: bool = False,
+) -> re.Pattern[str]:
+    """Compile env filter patterns into a single regex for ``fullmatch``.
+
+    Exact patterns (e.g. ``HOME``) become ``HOME`` and prefix patterns
+    (e.g. ``LC_*``) become ``LC_.*``.  The result is
+    ``HOME|LC_.*|...`` (used with ``fullmatch``).
+
+    *patterns* must be non-empty.
+    """
+    parts: list[str] = []
+    for p in patterns:
+        if p.endswith("*"):
+            parts.append(re.escape(p[:-1]) + ".*")
+        else:
+            parts.append(re.escape(p))
+    flags = re.IGNORECASE if case_insensitive else 0
+    return re.compile("|".join(parts), flags)
+
+
+class ExternalCommands(pydantic.BaseModel):
+    """Environment variable filtering for subprocesses.
+
+    Variables whose keys are not valid POSIX names are always removed.
+    A hard-coded set of variables required for basic subprocess
+    operation (see ``DEFAULT_KEEP_ENV``) is always kept.  User-supplied
+    ``keep_env`` patterns are evaluated before ``delete_env`` patterns.
+
+    ::
+
+      external_commands:
+        keep_env:
+          - "CARGO_*"
+        delete_env:
+          - "CI_TOKEN"
+          - "AWS_*"
+
+    .. versionadded:: 0.92.0
+    """
+
+    model_config = MODEL_CONFIG
+
+    DEFAULT_KEEP_ENV: typing.ClassVar[tuple[str, ...]] = (
+        "HOME",
+        "HOSTNAME",
+        "LANG",
+        "LANGUAGE",
+        "LC_*",
+        "LOGNAME",
+        "NO_COLOR",
+        "PATH",
+        "SHELL",
+        "TEMP",
+        "TERM",
+        "TMP",
+        "TMPDIR",
+        "TZ",
+        "USER",
+    )
+    """Patterns always kept regardless of user configuration."""
+
+    keep_env: list[KeepEnvPattern] = Field(default_factory=list)
+    """Allowlist patterns (evaluated before ``delete_env``)"""
+
+    delete_env: list[DeleteEnvPattern] = Field(default_factory=list)
+    """Blocklist patterns (evaluated after ``keep_env``)"""
+
+    _keep_re: re.Pattern[str] | None = PrivateAttr(default=None)
+    _delete_re: re.Pattern[str] | None = PrivateAttr(default=None)
+
+    @pydantic.model_validator(mode="after")
+    def validate_delete_env(self) -> typing.Self:
+        """Validate ``delete_env`` for conflicts and redundancy."""
+        if not self.delete_env:
+            return self
+        if "*" in self.delete_env and len(self.delete_env) > 1:
+            raise ValueError(
+                "delete_env: bare '*' must be the only entry, "
+                "additional patterns are redundant"
+            )
+        # Exact string overlap check.  This catches obvious
+        # configuration mistakes (e.g. ``delete_env: [HOME]``) but does
+        # not detect all conflicts — for example ``delete_env: [LC_ALL]``
+        # is not flagged even though ``LC_ALL`` matches the default keep
+        # pattern ``LC_*``.
+        keep = set(self.DEFAULT_KEEP_ENV) | set(self.keep_env)
+        overlap = keep & set(self.delete_env)
+        if overlap:
+            raise ValueError(
+                f"delete_env overlaps with keep_env / DEFAULT_KEEP_ENV: "
+                f"{sorted(overlap)}"
+            )
+        return self
+
+    def model_post_init(self, __context: typing.Any) -> None:
+        """Pydantic post init hook to initialize internal data structures"""
+        if self.delete_env:
+            self._keep_re = _compile_env_patterns(
+                self.DEFAULT_KEEP_ENV + tuple(self.keep_env)
+            )
+            if "*" not in self.delete_env:
+                self._delete_re = _compile_env_patterns(
+                    tuple(self.delete_env), case_insensitive=True
+                )
+
+    def filter_env(self, env: Mapping[str, str]) -> Mapping[str, str]:
+        """Filter environment variables by keep/delete patterns.
+
+        Variables whose keys are not valid POSIX names are always
+        removed first.  Of the remaining variables, those matching
+        ``DEFAULT_KEEP_ENV`` or ``keep_env`` are always kept.  Of the
+        rest, those matching ``delete_env`` are removed.  Variables
+        matching neither list are kept.
+        """
+        # Remove keys that are not valid POSIX names, e.g. keys with
+        # dashes, dots, spaces, or bash function exports (BASH_FUNC_*%%).
+        env = {k: v for k, v in env.items() if _POSIX_ENV_KEY_RE.fullmatch(k)}
+        # _keep_re is only set in model_post_init when delete_env is
+        # non-empty, so None means no filtering is configured.
+        if self._keep_re is None:
+            return env
+        if self._delete_re is None:
+            # delete_env is ["*"]: keep only what matches
+            return {k: v for k, v in env.items() if self._keep_re.fullmatch(k)}
+        return {
+            k: v
+            for k, v in env.items()
+            if self._keep_re.fullmatch(k) or not self._delete_re.fullmatch(k)
+        }
+
+
 class PurlConfig(pydantic.BaseModel):
     """Per-package purl configuration for SBOM generation.
 

src/fromager/packagesettings/_settings.py

@@ -13,7 +13,7 @@
 from pydantic import Field
 
 from .. import overrides
-from ._models import PackageSettings, SbomSettings
+from ._models import ExternalCommands, PackageSettings, SbomSettings
 from ._pbi import PackageBuildInfo
 from ._typedefs import MODEL_CONFIG, GlobalChangelog, Package, Variant
 
@@ -45,6 +45,16 @@ class SettingsFile(pydantic.BaseModel):
     are generated.
     """
 
+    external_commands: ExternalCommands = Field(default_factory=ExternalCommands)
+    """Environment variable filtering for subprocesses
+
+    Controls which environment variables are passed to child processes
+    using ``keep_env`` / ``delete_env`` patterns.  Defaults to no
+    filtering.
+
+    .. versionadded:: 0.92.0
+    """
+
     @classmethod
     def from_string(
         cls,
@@ -175,6 +185,14 @@ def sbom_settings(self) -> SbomSettings | None:
         """Get global SBOM settings, or None if SBOM generation is disabled."""
         return self._settings.sbom
 
+    @property
+    def external_commands(self) -> ExternalCommands:
+        """Get external commands settings.
+
+        .. versionadded:: 0.92.0
+        """
+        return self._settings.external_commands
+
     def variant_changelog(self) -> list[str]:
         """Get global changelog for current variant"""
         return list(self._settings.changelog.get(self.variant, []))