Skip to content
Merged
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
30 changes: 20 additions & 10 deletions docs/proposals/filter-env.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment thread
rd4398 marked this conversation as resolved.
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.
Expand Down
2 changes: 2 additions & 0 deletions src/fromager/packagesettings/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from ._models import (
BuildOptions,
DownloadSource,
ExternalCommands,
GitOptions,
PackageSettings,
ProjectOverride,
Expand Down Expand Up @@ -57,6 +58,7 @@
"DownloadSource",
"EnvKey",
"EnvVars",
"ExternalCommands",
"GitHubTagCloneResolver",
"GitHubTagDownloadResolver",
"GitLabTagCloneResolver",
Expand Down
158 changes: 157 additions & 1 deletion src/fromager/packagesettings/_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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_]*\*?)$"),
Comment thread
smoparth marked this conversation as resolved.
]


# 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):
Comment thread
tiran marked this conversation as resolved.
Comment thread
smoparth marked this conversation as resolved.
"""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, ...]] = (
Comment thread
smoparth marked this conversation as resolved.
"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:
Comment thread
smoparth marked this conversation as resolved.
raise ValueError(
f"delete_env overlaps with keep_env / DEFAULT_KEEP_ENV: "
f"{sorted(overlap)}"
)
return self
Comment thread
coderabbitai[bot] marked this conversation as resolved.

def model_post_init(self, __context: typing.Any) -> None:
Comment thread
tiran marked this conversation as resolved.
"""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]:
Comment thread
smoparth marked this conversation as resolved.
"""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)}
Comment thread
smoparth marked this conversation as resolved.
# _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.

Expand Down
20 changes: 19 additions & 1 deletion src/fromager/packagesettings/_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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, []))
Expand Down
Loading
Loading