diff --git a/CHANGELOG.md b/CHANGELOG.md index b0c463527..5de747b67 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -262,6 +262,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **The `fluid forge` welcome scan kept walking the whole directory tree after it had returned.** + Its type-suggestion probe listed contracts with `sorted(cwd.rglob("contract.fluid.yaml"))[:8]`, + which reads every file under the current directory (279 s under a folder holding many repos) and + goes into `.venv`, `.git` and `node_modules`. The scan returns at its 1.0 s budget, but Python + cannot kill a thread, so the walker went on at about half a core, and the mode picker, the + interview and the welcome panel each start a scan: up to three overlapped. The walk is now + bounded and ends with its scan. It skips the directories `util/upstream_discovery` already skips + (`.git`, `.venv`, `site-packages`, `node_modules`, `dist`, `build`, ...), lists at most 6 levels + below the current directory, reads at most 10,000 directory entries, and stops at its next + directory once the scan has returned; one walk runs at a time per process. It tallies the same + first 8 contracts as before wherever those limits do not cut the walk. A symlink named + `contract.fluid.yaml`, or one that is not a regular file (a FIFO, a device), is no longer + opened. + - **`fluid apply --mode amend-and-build` provisioned a product, then refused its id.** The build phase keeps run records, cursors and watermarks under `.fluid/runs///`, so its ids must start with a letter or underscore, use only letters, digits, `_`, `.` and `-`, @@ -591,6 +605,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security +- **A contract chose both the variable and the hosts an Iceberg sink's catalog credential went + to.** On the Kafka Connect and embedded Debezium Iceberg sinks, `sink.catalogAuth` names an + environment variable (`tokenEnv` / `credentialEnv`), and the same contract names the catalog + (`binding.location.uri`) and the OAuth2 token endpoint (`oauth2ServerUri`). The config forge-cli + pushes or writes carries `env:`, which Iceberg resolves on the Connect worker, or on + embedded Debezium from the runner's whole environment, so any variable there could be sent to a + host the contract picked. Every host the credential reaches must now be https (a loopback one + included) and allowed by the operator: the variable paired with it as `NAME@host` in + `FLUID_ACQUISITION_SECRET_REF_ALLOWLIST`, and a hostname listed in + `FLUID_ACQUISITION_DEPLOYMENT_HOST_ALLOWLIST`, because the worker resolves the name itself, + later. An IP address keeps that list's rules (link-local never, a private one with no entry, a + public one only when listed), and a loopback host needs neither entry. A bare `NAME` entry + allows every host and a listed hostname also covers its subdomains, so prefer `NAME@host` and + the exact host. The run refuses before any Connect REST call, `application.properties` write or + read of the variable; `fluid validate` warns with the `export` line the run needs, and refuses + an `http://` or link-local host outright. `binding.location` text that reaches the derived sink + config may no longer start with `env:` or hold `${` or a control character, and its `region` + must be one host label. **Upgrade:** a sink whose `catalogAuth` reaches a non-loopback host + fails its run until the operator adds both entries, and one on an `http://` catalog or token + endpoint fails `fluid validate`. A derived sink whose `binding.location` holds such text, or a + region that is not one label, now fails `fluid validate` and the run, `catalogAuth` or not; + write the value itself. + - **`validate_ident` let a name with a trailing newline through.** Python's `$` also matches just before a trailing newline, so the identifier allowlist in `providers/_sql_safety.py`, which the DDL and OpenTofu emitters route names through, accepted `orders\n` and passed it on with the @@ -621,6 +658,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 project dotenv file cannot point them elsewhere while `FLUID_LLM_API_KEY` or a stored key is in play, as for every other provider endpoint. +- **The litellm endpoint check could not see a provider's per-kind route.** It looked for names + ending in `_API_BASE`, so the nine `DASHSCOPE_`, `QWENCLOUD_` and `QWEN_AI_PLATFORM_API_BASE_IMAGE`, + `_RERANK` and `_VIDEO` names litellm reads (three of them new in 1.108.0) were never listed, and + a project dotenv file could set them unpaired. They are listed now and the check reads a kind + after `_API_BASE`. FLUID calls only `litellm.completion`, which none of them routes, so nothing + reached them. The Databricks model-serving group also names `DATABRICKS_CLIENT_SECRET`, which + litellm's OAuth M2M path posts to the workspace `DATABRICKS_API_BASE` names; the name-shape rule + already refused that pairing, and the group now says why. + ### Upgrade notes - New validate warning, so `--strict` fails: a contract with builds whose `contract.id` or a diff --git a/CLAUDE.md b/CLAUDE.md index 551f490d7..5ea36c5e8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -420,10 +420,11 @@ hand-rolling the protocol. `cli/_welcome_scan.py::run_welcome_scan` now has a budget that means what it says: -- **One wall-clock cap.** `_TOTAL_TIMEOUT_S` (1.0 s) is the only scan budget; the old per-probe `_PROBE_TIMEOUT_S` is gone. Probes run on daemon threads (`_spawn_probe`: one `Future` each, deliberately not a `ThreadPoolExecutor`, whose non-daemon workers are joined at interpreter exit and would hold the CLI open for an abandoned probe). The scan returns at the deadline and drops stragglers. Python cannot kill a thread, so an abandoned probe keeps running in the background until it finishes; the one that can run long is `_probe_specialization_suggestion` (unbounded `rglob`), and the picker, the interview signals and the welcome panel each start a scan, so up to three walkers can overlap on a huge cwd. Probes must not write to stdout/stderr (a daemon thread caught mid-write at interpreter exit can abort the process). +- **One wall-clock cap.** `_TOTAL_TIMEOUT_S` (1.0 s) is the only scan budget; the old per-probe `_PROBE_TIMEOUT_S` is gone. Probes run on daemon threads (`_spawn_probe`: one `Future` each, deliberately not a `ThreadPoolExecutor`, whose non-daemon workers are joined at interpreter exit and would hold the CLI open for an abandoned probe). The scan returns at the deadline and drops stragglers. Python cannot kill a thread, so an abandoned probe keeps running until it finishes (the one that used to run for minutes is the tree walk, below). Probes must not write to stdout/stderr (a daemon thread caught mid-write at interpreter exit can abort the process). - **Cold imports are off the clock.** `_warm_probe_imports` imports `_WARM_MODULES` (the AI-config pydantic chain, the dbt runner) from the main thread while the probes run, then the budget starts. Probe imports stay function-local: if a probe starts importing something new, list it in `_WARM_MODULES` or `tests/test_welcome_scan_budget.py::test_probes_import_nothing_new_after_the_warm_up` fails. NB `build_parser()` already imports `ai_setup`, the dbt runner and pydantic, so in a real CLI process only `copilot.unified_config` is still cold; the warm-up matters for tests and library callers. - **dbt has its own cap.** `_DBT_VERSION_TIMEOUT_S` (0.32 s) sizes the `dbt --version` subprocess and is not derived from the scan budget and must stay below it, so the child is killed by its own timeout instead of being orphaned. With Python dbt-core on PATH it usually waits out that cap, which is the scan's floor (~0.35 s). -- `scan_duration_ms` is the real wall-clock of the call, imports included. Pins: `tests/test_welcome_scan_budget.py`. +- **The specialization walk is bounded and ends with its scan.** `_probe_specialization_suggestion` used to `rglob` the whole cwd tree (279 s under a directory holding many repos), and the picker, the interview signals and the welcome panel each start a scan, so up to three abandoned walkers overlapped (~0.5 core). Its contracts now come from `_find_contracts`: depth-first in name order, so the first 8 are exactly `sorted(rglob(...))[:8]` wherever the bounds do not cut. It skips `util/upstream_discovery`'s `IGNORED_DIRS | BUILD_OUTPUT_DIRS`, lists at most `_SPECIALIZATION_MAX_DEPTH` (6) levels, reads at most `_SPECIALIZATION_MAX_ENTRIES` (10,000) entries, and does not follow symlinks or take anything but a regular file. `run_welcome_scan` sets a `threading.Event` when it returns and the walk ends at its next directory; `_WALK_GATE` runs one walk at a time, so a walker stuck in a filesystem call (a dead mount) is the only one a process can hold. `tests/fragments/test_no_raw_readers.py` registers `_probe_specialization_suggestion` as an accepted raw reader. The scanner finds it through the word `contract` in that function (the `contract.fluid.yaml` in its docstring, the `_find_contracts` call); the registry only checks that the function exists, so losing both silently orphans the entry. +- `scan_duration_ms` is the real wall-clock of the call, imports included. Pins: `tests/test_welcome_scan_budget.py`, `tests/test_welcome_scan_walk.py`. ## Working style diff --git a/fluid_build/build_runners/_secret_policy.py b/fluid_build/build_runners/_secret_policy.py index 28112c998..729d91af2 100644 --- a/fluid_build/build_runners/_secret_policy.py +++ b/fluid_build/build_runners/_secret_policy.py @@ -197,8 +197,9 @@ def variable_allowed( ) -> bool: """Whether ``variable`` may be sent to ``host``: the decision, without wording. - For a caller that phrases its own refusal (the managed Flink lane's catalog - token, ``streaming/policy.py``). A loopback host needs no entry. + For a caller that phrases its own refusal: the managed Flink lane's catalog + token (``streaming/policy.py``) and the Iceberg sinks' catalog token + (``kafka_connect/iceberg_sink_validation.py``). A loopback host needs no entry. """ return is_loopback(host) or _allows(variable, host, allowlist(environ)) diff --git a/fluid_build/build_runners/debezium/runner.py b/fluid_build/build_runners/debezium/runner.py index 4f24ad570..4fb0dec76 100644 --- a/fluid_build/build_runners/debezium/runner.py +++ b/fluid_build/build_runners/debezium/runner.py @@ -706,7 +706,8 @@ def _execute_debezium_server( # The server inherits this process's environment, which is where Iceberg # resolves the ``env:`` catalog properties ``sink.catalogAuth`` - # emits, and where the preflight checked the variable is set. The + # emits, and where the preflight checked the variable is set, once the + # operator had paired it with every host it reaches. The # ``FLUID_DBZ_*`` credentials the file refers to are added to it. No # secret is written to application.properties. try: diff --git a/fluid_build/build_runners/kafka_connect/iceberg_sink_validation.py b/fluid_build/build_runners/kafka_connect/iceberg_sink_validation.py index 9c9a2ffa6..f213f31df 100644 --- a/fluid_build/build_runners/kafka_connect/iceberg_sink_validation.py +++ b/fluid_build/build_runners/kafka_connect/iceberg_sink_validation.py @@ -63,7 +63,17 @@ properties it emits; an embedded Debezium run also needs its variable set (the Kafka Connect variable lives on the worker, which forge-cli cannot see); and an override map that inlines a ``credential`` or ``token`` value - is a warning. + is a warning; +* every host a ``sink.catalogAuth`` credential reaches (the catalog, and an + OAuth2 token endpoint) must be https and must not be link-local, an error + here and in the run's preflight; and the operator must allow it: the variable + paired with the host (``NAME@host``), and a hostname listed, because the + worker or the server resolves it itself. Those lists are the operator's, so + a missing entry is a warning here and an error in the preflight, which + checks it before the variable is read; +* the ``binding.location`` text a derived sink config carries must not start + with ``env:`` or hold ``${`` or a control character, and ``client.region`` + must be one host label. Debezium builds in ``bring-your-own`` / ``managed`` mode create only the SOURCE connector (no sink is derived), so none of this applies to them; an embedded @@ -86,8 +96,10 @@ import contextlib import contextvars +import ipaddress import logging import os +import re from dataclasses import dataclass from typing import ( AbstractSet, @@ -100,9 +112,13 @@ Optional, Sequence, Tuple, + Union, ) +from urllib.parse import urlsplit +from ..._net import is_public_address from ...providers._iceberg_catalog import ( + BIGQUERY_LOCATION, BIGQUERY_PROJECT_ID, BUCKET_WAREHOUSE_KINDS, DYNAMODB_CATALOG_IMPL, @@ -111,6 +127,7 @@ FAMILY_UNKNOWN, GLUE_CATALOG_IMPL, CatalogKind, + ResolvedIcebergCatalog, binding_catalog_kind, canonical_catalog_kind, catalog_kind_info, @@ -125,12 +142,20 @@ AUTH_OWNED_PROPERTIES, AUTHORIZATION_HEADER_PROPERTY, CATALOG_AUTH_FIELD, + FORBIDDEN_TEXT_PATTERN, INLINE_CHECKED_KEYS, CatalogAuth, is_literal_secret, parse_catalog_auth, raw_catalog_auth, ) +from ...util.binding_paths import ENV_PLACEHOLDER_RE +from .._deployment_host_policy import ALLOWLIST_ENV as HOST_ALLOWLIST_ENV +from .._deployment_host_policy import _on_allowlist +from .._deployment_host_policy import allowlist as host_allowlist +from .._secret_policy import ALLOWLIST_ENV as SECRET_REF_ALLOWLIST_ENV +from .._secret_policy import allowlist as secret_ref_allowlist +from .._secret_policy import is_loopback, variable_allowed LOG = logging.getLogger("fluid.acquire.iceberg_sink") @@ -436,7 +461,7 @@ def _check_build( "streamingSink.routeField" ) - _check_catalog_auth(bid, sink, runtime, targets, errors) + _check_catalog_auth(bid, sink, runtime, targets, errors, warnings) _check_inlined_secrets(bid, sink, runtime, warnings) # The catalog checks read each expose the sink writes. A Debezium Server @@ -506,6 +531,8 @@ def _check_catalog( f"use one of: {', '.join(known_catalog_kinds())}" ) return + if runtime.derives and runtime.engine in _CATALOG_AUTH_ENGINES: + _check_location_text(bid, binding, sink, runtime, errors) # The sink must write through the catalog dbt and the IaC read. Both read # only the expose, so a ``sink.catalog``, an override or a hand-written @@ -1136,9 +1163,10 @@ def _check_catalog_auth( runtime: IcebergSinkPlan, targets: Sequence[Mapping[str, Any]], errors: List[str], + warnings: List[str], ) -> None: - """``sink.catalogAuth``: shape, catalog, derivation, overrides, and the - embedded Debezium variable at run time. HARD.""" + """``sink.catalogAuth``: shape, catalog, derivation, overrides, the hosts it + reaches, and the embedded Debezium variable at run time. HARD.""" raw = raw_catalog_auth(sink) if raw is None: return @@ -1214,7 +1242,7 @@ def _check_catalog_auth( f"keys from {label}, or remove sink.{CATALOG_AUTH_FIELD}" ) - _check_catalog_auth_env_at_run(head, auth, runtime, errors) + _check_catalog_auth_reach(head, auth, refused, runtime, targets, errors, warnings) def _check_catalog_auth_env_at_run( @@ -1252,6 +1280,298 @@ def _check_catalog_auth_env_at_run( run_only.append(issue) +#: The environment each engine resolves ``sink.catalogAuth``'s ``env:`` from. +_RESOLVED_FROM: Mapping[str, str] = { + "kafka-connect": "the Kafka Connect worker's environment", + "debezium": "this runner's environment, which the embedded Debezium Server inherits,", +} + +_OPERATOR_ONLY = ( + "in the OPERATOR environment (a shell or CI variable, not a project .env, which is " + "ignored for these settings)" +) + + +#: A URL's scheme and authority: what decides where a request goes. +_URL_HEAD_RE = re.compile(r"[^/]*(?://[^/?#]*)?") + + +def _rendered_at_run(value: str) -> bool: + """Under ``fluid validate``, does ``value`` wait on a ``{{ env.* }}`` template? + + The acquisition dispatcher renders every such template before a runner + and its preflight see the contract (``build_runners/base.py``), so the + preflight checks the value the sink receives. ``fluid validate`` sees the + template itself and leaves the check to the run, but only when the + template is where it changes the answer: in a URL's scheme or authority, + not in its path. + """ + if _AT_RUN.get() is not None: + return False + head = _URL_HEAD_RE.match(value) + return bool(ENV_PLACEHOLDER_RE.search(head.group(0) if head else value)) + + +def _catalog_uri_reaching(binding: Mapping[str, Any], runtime: IcebergSinkPlan) -> Tuple[str, str]: + """The catalog uri the sink's REST client calls, and the setting that names it. + + ``binding.location.uri``, unless an override map merged over the derived + config sets the uri property: the last one wins, as the runner merges them. + The credential goes wherever this points. + """ + key = f"{_property_prefix(runtime)}uri" + uri = str((binding.get("location") or {}).get("uri") or "") + source = "binding.location.uri" + for label, mapping in runtime.overrides: + if key in mapping: + uri, source = str(mapping[key] or ""), f"{label} {key}" + return uri, source + + +def _ip_literal(host: str) -> Optional[Union[ipaddress.IPv4Address, ipaddress.IPv6Address]]: + """``host`` as an address, an IPv4-mapped IPv6 one as its IPv4; ``None`` for a name. + + Parsed as written: ``10.0.0.1%25.example`` is a name, not 10.0.0.1. + """ + try: + address = ipaddress.ip_address(host) + except ValueError: + return None + if isinstance(address, ipaddress.IPv6Address) and address.ipv4_mapped is not None: + return address.ipv4_mapped + return address + + +def _check_catalog_auth_reach( + head: str, + auth: Optional[CatalogAuth], + refused: Sequence[str], + runtime: IcebergSinkPlan, + targets: Sequence[Mapping[str, Any]], + errors: List[str], + warnings: List[str], +) -> None: + """The hosts the credential reaches, then the embedded Debezium variable. + + In that order, so a refused destination never reads the variable. + ``refused`` names the catalog kinds not reached over REST: that is already + an error, and nothing there reads the credential, so nothing reads the + variable either. + """ + if auth is None or refused: + return + if not _check_catalog_auth_destinations(head, auth, runtime, targets, errors, warnings): + _check_catalog_auth_env_at_run(head, auth, runtime, errors) + + +def _credential_destinations( + auth: CatalogAuth, runtime: IcebergSinkPlan, targets: Sequence[Mapping[str, Any]] +) -> List[Tuple[str, str, str]]: + """``(role, setting, url)`` for each host the credential reaches. + + The catalog the sink calls (:func:`_catalog_uri_reaching`), once per uri, + and for ``oauth2`` the token endpoint, which receives the credential itself + while the catalog receives a token minted from it. + """ + destinations: List[Tuple[str, str, str]] = [] + for target in targets: + uri, source = _catalog_uri_reaching(target.get("binding") or {}, runtime) + if uri.strip() and all(uri != url for _role, _source, url in destinations): + destinations.append(("Iceberg catalog", source, uri)) + if auth.type == "oauth2": + source = f"sink.{CATALOG_AUTH_FIELD}.oauth2ServerUri" + destinations.append(("OAuth2 token endpoint", source, auth.oauth2_server_uri)) + return destinations + + +def _received(auth: CatalogAuth, role: str) -> str: + """What the ``role`` host receives of the credential, for the messages.""" + if role == "OAuth2 token endpoint": + return f"the credential in {auth.env_var} (the client id and secret)" + if auth.type == "oauth2": + return f"a token minted from the credential in {auth.env_var}" + return f"the token in {auth.env_var}" + + +#: A host the operator can list: a DNS name (``urlsplit`` has lowercased it) or +#: an IP address, in plain characters. The refusal's ``export`` line repeats +#: the host, so a quote, ``$(`` or comma there would turn the operator's paste +#: into a command, or into a bare entry that lets some other variable go to any +#: host. A ``%`` zone id names an interface, never a remote catalog. +_PLAIN_HOST_RE = re.compile(r"[a-z0-9._-]+|[0-9a-f:.]+") + + +def _not_https(source: str, role: str, receives: str) -> str: + return ( + f"{source} must be an https:// URL: the {role} receives {receives} on every " + "request, and http would carry it in clear text, so a loopback host needs https too" + ) + + +def _destination_host(url: str, source: str, role: str, receives: str) -> Tuple[str, str]: + """``(host, "")`` for a destination the operator may allow, else ``("", why not)``. + + "Why not" is a refusal no operator entry changes: the URL is not https (a + loopback one included), carries a ``user:password@`` part, names no plain + host (:data:`_PLAIN_HOST_RE`), or names a link-local or unspecified + address, where cloud instance metadata lives (the deployment host policy's + rule). Python and the JVM split a URL's authority alike only when it holds + no such part, so the host checked here is the one the sink connects to. + """ + no_host = f"{source} names no host, so where the {role} sends {receives} cannot be checked" + try: + parts = urlsplit(url) + host = parts.hostname or "" + except ValueError: # an unbalanced "[" in the netloc: no host can be read + return "", no_host + if parts.scheme != "https": + return "", _not_https(source, role, receives) + if "@" in parts.netloc: + return "", ( + f"{source} carries a user:password@ part (not shown). Remove it: the {role} " + f"receives {receives}, and a credential belongs in that variable, not in the URL" + ) + if not host: + return "", no_host + if not _PLAIN_HOST_RE.fullmatch(host): + return "", ( + f"{source} names a host that is not a plain DNS name or IP address (letters, " + "digits, '.', '-' and '_'; a non-ASCII name in its xn-- form; no zone id). It " + f"is not shown, and the {role} would receive {receives}" + ) + address = _ip_literal(host) + if address is not None and (address.is_link_local or address.is_unspecified): + return "", ( + f"the {role} host {host!r} ({source}) is link-local or unspecified, where cloud " + "instance metadata lives; this is refused even with an allowlist entry" + ) + return host, "" + + +def _unlisted_host(host: str) -> Optional[str]: + """Why ``host`` needs a host-list entry the operator has not given, or ``None``. + + The rule the founder chose for these sinks on 2026-11-04. The Kafka Connect + worker or the Debezium Server + resolves a hostname itself, after this check, so no DNS answer seen here + shows where it will connect: a name passes only when the operator lists it + in ``FLUID_ACQUISITION_DEPLOYMENT_HOST_ALLOWLIST``, or when it is + ``localhost``. An IP literal keeps the deployment host policy's rules + (``_deployment_host_policy.py``): a private or loopback address needs no + entry, a public one is listed (link-local is refused before this, by + :func:`_destination_host`). Nothing is resolved. A listed host is matched by + that policy's own suffix rule, so the two lists cannot read differently. + """ + if _on_allowlist(host, host_allowlist()): + return None + if _ip_literal(host) is not None: + return ( + f"is a public address not in {HOST_ALLOWLIST_ENV}" if is_public_address(host) else None + ) + return None if is_loopback(host) else f"is a name not in {HOST_ALLOWLIST_ENV}" + + +def _check_catalog_auth_destinations( + head: str, + auth: CatalogAuth, + runtime: IcebergSinkPlan, + targets: Sequence[Mapping[str, Any]], + errors: List[str], + warnings: List[str], +) -> bool: + """Every host the credential reaches must be https and allowed by the operator. + + The contract names the variable (``tokenEnv`` / ``credentialEnv``) and the + hosts (:func:`_credential_destinations`). Iceberg resolves ``env:`` + where the sink runs (on embedded Debezium, from this runner's whole + environment) and sends the value there, so without an operator boundary any + variable there could go to a host the contract chose. Each host needs: + + * https, and an address no entry has to rule out (:func:`_destination_host`). + HARD; + * the variable paired with it in ``FLUID_ACQUISITION_SECRET_REF_ALLOWLIST`` + (``NAME@host``), unless it is loopback (``_secret_policy.variable_allowed``); + * a hostname listed in ``FLUID_ACQUISITION_DEPLOYMENT_HOST_ALLOWLIST`` + (:func:`_unlisted_host`). + + Both lists are operator-only (``credentials/protected_env.py`` ignores a + project dotenv file's value) and are read where the sink config is + derived: the run's preflight refuses, and ``fluid validate``, which may run + without them, warns with the entries the run needs. Returns ``True`` when + the run must stop. + """ + hard: List[str] = [] + reasons: List[str] = [] + pairs: List[str] = [] + hosts: List[str] = [] + for role, source, url in _credential_destinations(auth, runtime, targets): + if _rendered_at_run(url): + # A template decides the host, which the run checks once rendered; + # a scheme written out is decided now. + scheme, written, _rest = url.partition("://") + if written and not ENV_PLACEHOLDER_RE.search(scheme): + if scheme.strip().lower() != "https": + hard.append(f"{head}: {_not_https(source, role, _received(auth, role))}") + continue + host, problem = _destination_host(url, source, role, _received(auth, role)) + if problem: + hard.append(f"{head}: {problem}") + continue + unlisted = _unlisted_host(host) + if unlisted is not None: + reasons.append(f"the {role} host {host!r} {unlisted}") + hosts.append(host) + if not variable_allowed(auth.env_var, host): + reasons.append( + f"{auth.env_var} is not paired with the {role} host {host!r} in " + f"{SECRET_REF_ALLOWLIST_ENV}" + ) + pairs.append(f"{auth.env_var}@{host}") + errors.extend(hard) + if reasons: + issue = _unallowed_destinations(head, auth, runtime, reasons, pairs, hosts) + run_only = _AT_RUN.get() + if run_only is None: + warnings.append(issue) + else: + errors.append(issue) + run_only.append(issue) + return bool(hard or reasons) + + +def _unallowed_destinations( + head: str, + auth: CatalogAuth, + runtime: IcebergSinkPlan, + reasons: Sequence[str], + pairs: Sequence[str], + hosts: Sequence[str], +) -> str: + """The refusal of the destinations the operator has yet to allow, with one export.""" + settings: List[str] = [] + if pairs: + # Rebuilt from the parsed list, so an entry the parser ignores is not echoed back. + current = [f"{n}@{h}" if h else n for n, h in secret_ref_allowlist()] + entries = ",".join(dict.fromkeys([*current, *pairs])) + settings.append(f'{SECRET_REF_ALLOWLIST_ENV}="{entries}"') + if hosts: + entries = ",".join(dict.fromkeys([*host_allowlist(), *hosts])) + settings.append(f'{HOST_ALLOWLIST_ENV}="{entries}"') + return ( + f"{head}: sink.{CATALOG_AUTH_FIELD}.{auth.env_field} names {auth.env_var}. Iceberg " + f"resolves it from {_RESOLVED_FROM[runtime.engine]} and sends it to hosts this " + "contract names, and the run refuses until the operator allows each: " + f"{'; '.join(dict.fromkeys(reasons))}. A contract names both the variable and the " + "hosts, so without the operator's lists any variable there could go to a host the " + "contract chose; a hostname needs an entry because the sink resolves it itself, " + "later, where no lookup made here applies. If this is intended, add the entries " + f"{_OPERATOR_ONLY}; this keeps the ones set here: export {' '.join(settings)}. Leave " + "the port off: a host written " + "with a port never matches. A loopback host needs no entry, and a private IP address " + "needs only the pairing" + ) + + def _check_inlined_secrets( bid: Any, sink: Mapping[str, Any], @@ -1287,3 +1607,136 @@ def _check_inlined_secrets( f"Use sink.{CATALOG_AUTH_FIELD}, or set {key} to env:, which Iceberg " "resolves from the environment of the process that runs the sink" ) + + +# --------------------------------------------------------------------------- +# binding.location text in the derived sink config +# --------------------------------------------------------------------------- + +#: One DNS label (RFC 1123): letters, digits and inner hyphens, at most 63. +#: Any case, because the sinks pass the region for every catalog kind and a +#: label ignores case. +_HOST_LABEL_RE = re.compile(r"[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?") + +#: The binding setting each catalog-specific property is derived from. +_EXTRA_PROPERTY_SOURCES: Mapping[str, str] = { + BIGQUERY_PROJECT_ID: "binding.location.project", + BIGQUERY_LOCATION: "binding.location.region", +} + +#: Who resolves a reference in the derived config, for the location messages. +_REFERENCES_RESOLVED_BY: Mapping[str, str] = { + "kafka-connect": ( + "the Kafka Connect worker, where Iceberg's REST catalog replaces a value that starts " + "with env: by that variable from the worker's environment and Kafka Connect resolves " + "a ${provider:key} expression with the worker's config providers" + ), + "debezium": ( + "the embedded Debezium Server, where Iceberg's REST catalog replaces a value that " + "starts with env: by that variable and SmallRye Config expands a ${...} expression, " + "both from the server's environment, which is this runner's" + ), +} + + +def _derived_location_text( + binding: Mapping[str, Any], + sink: Mapping[str, Any], + resolved: ResolvedIcebergCatalog, + runtime: IcebergSinkPlan, +) -> List[Tuple[str, str]]: + """``(setting, value)`` for each contract value the derived config carries. + + Read off the resolved catalog both emitters build from, so a value is + checked as it reaches the sink, a warehouse derived from a bucket included, + and only the field the sink reads where several could name it: the first + namespace field set, as ``iceberg_namespace`` reads them, and + ``sink.partitionBy`` over ``binding.location.partitionBy``, as the runner's + ``SinkSpec`` does. Kafka Connect also names the table; Debezium Server + composes it. + """ + loc = binding.get("location") or {} + text: List[Tuple[str, str]] = [] + if resolved.uri: + text.append(("binding.location.uri", str(resolved.uri))) + if resolved.warehouse: + written = str(loc.get("warehouse") or "").strip() + source = ( + "binding.location.warehouse" + if written and resolved.warehouse == written + else "the warehouse derived from binding.location (bucket, path, database, table)" + ) + text.append((source, resolved.warehouse)) + if resolved.region: + text.append(("binding.location.region", str(resolved.region))) + for prop, value in resolved.extra_catalog_props.items(): + text.append((_EXTRA_PROPERTY_SOURCES.get(prop, f"binding.location ({prop})"), str(value))) + namespaces = ( + ("binding.location.database", loc.get("database")), + ("binding.database", binding.get("database")), + ("binding.location.dataset", loc.get("dataset")), + ) + tables = (("binding.location.table", loc.get("table")), ("binding.table", binding.get("table"))) + used = [next(((s, v) for s, v in namespaces if v), None)] + if runtime.engine == "kafka-connect": # Debezium Server composes the table itself + used.append(next(((s, v) for s, v in tables if v), None)) + text.extend((source, str(value)) for source, value in filter(None, used)) + raw_partition = sink.get("partitionBy") + try: # as SinkSpec.from_dict reads it: a mapping gives its keys, a string its letters + partition = list(raw_partition) if raw_partition else [] + except TypeError: # SinkSpec refuses it too, so the run stops before any config + partition = [] + if partition: + text.extend(("sink.partitionBy", str(item)) for item in partition) + else: + text.extend(("binding.location.partitionBy", str(item)) for item in resolved.partition_by) + return text + + +def _check_location_text( + bid: Any, + binding: Mapping[str, Any], + sink: Mapping[str, Any], + runtime: IcebergSinkPlan, + errors: List[str], +) -> None: + """The ``binding.location`` text a derived sink config carries. HARD. + + Iceberg's REST catalog replaces a catalog property that starts with ``env:`` + by that variable, in the process that builds the catalog + (EnvironmentUtil.resolveAll, from RESTSessionCatalog.initialize: + apache-iceberg-1.10.0 RESTSessionCatalog.java:195). A warehouse is then sent + to the catalog and a region becomes part of the S3 and Glue endpoint hosts, + so location text that references a variable could send any variable the + worker or the server holds to a host the contract chose; ``${`` is the Kafka + Connect and SmallRye form of the same reference, read for every catalog + kind. The ``env:`` rule is applied to every kind too, as hardening: no real + value starts with it, and a kind that later resolves it needs no new check. + Only ``sink.catalogAuth`` names a variable, and the operator pairs it with + its hosts (:func:`_check_catalog_auth_destinations`). A value is never + quoted: it names a variable. ``client.region`` must also be one host label. + """ + resolved = resolve_iceberg_catalog(binding, sink=sink, account_ref="") + referenced: set[str] = set() + for source, value in _derived_location_text(binding, sink, resolved, runtime): + if not re.search(FORBIDDEN_TEXT_PATTERN, value) or source in referenced: + continue + referenced.add(source) + errors.append( + f"iceberg sink (build {bid!r}): {source} starts with env:, or holds ${{ or a " + "control character (the value is not shown). The derived sink config hands it to " + f"{_REFERENCES_RESOLVED_BY[runtime.engine]}, so it could send any variable there to " + f"a host the contract chose. Write the value itself; only sink.{CATALOG_AUTH_FIELD} " + "may name a variable, and the operator pairs it with the hosts it reaches" + ) + region = str(resolved.region or "") + if region and "binding.location.region" not in referenced and not _rendered_at_run(region): + if not _HOST_LABEL_RE.fullmatch(region): + errors.append( + f"iceberg sink (build {bid!r}): binding.location.region is not a region name " + "such as eu-central-1 (the value is not shown: at run it may have come from a " + "{{ env.* }} template). The derived sink config passes it to " + "Iceberg's AWS client as client.region, which builds the S3 and Glue endpoint " + "hosts from it, so it must be one host label: letters, digits and inner " + "hyphens, at most 63 characters" + ) diff --git a/fluid_build/build_runners/kafka_connect/runner.py b/fluid_build/build_runners/kafka_connect/runner.py index fee82975c..7f8eb0225 100644 --- a/fluid_build/build_runners/kafka_connect/runner.py +++ b/fluid_build/build_runners/kafka_connect/runner.py @@ -440,6 +440,8 @@ def _execute(ctx: RunContext, runner: KafkaConnectRunner) -> RunResult: # ``env:`` references only; the variable lives on the # Connect worker, out of this process's sight, so there is no # run check for it here (the embedded Debezium runner has one). + # The preflight above refused it unless the operator pairs it + # with every host it reaches. catalog_auth=catalog_auth_of(props.get("sink")), ) # A bucket whose variable is unset or empty here resolved to one the diff --git a/fluid_build/cli/_welcome_scan.py b/fluid_build/cli/_welcome_scan.py index 4c2f58e14..abfd5eaa1 100644 --- a/fluid_build/cli/_welcome_scan.py +++ b/fluid_build/cli/_welcome_scan.py @@ -31,8 +31,10 @@ Every probe is best-effort and never raises. Budget: once the one-time module imports are done, the probes share ``_TOTAL_TIMEOUT_S`` of wall-clock -and the scan returns by then, dropping any probe still running. The dbt probe -sets the floor: with Python dbt-core on PATH it usually waits out +and the scan returns by then, dropping any probe still running. A dropped probe +is abandoned, not killed (Python cannot kill a thread); the one that walks the +directory tree bounds itself and ends when the scan returns (``_find_contracts``). +The dbt probe sets the floor: with Python dbt-core on PATH it usually waits out ``_DBT_VERSION_TIMEOUT_S`` and reports nothing. ``scan_duration_ms`` is the measured wall-clock of the whole call, imports included. """ @@ -48,6 +50,7 @@ import time from concurrent.futures import Future, wait from dataclasses import asdict, dataclass, field +from itertools import islice from pathlib import Path from typing import Any, Callable, Dict, List, Optional, Tuple @@ -73,11 +76,30 @@ # the AI probe's cold cost (~0.1s CPU, several times that on a loaded host). "fluid_build.copilot.unified_config", "fluid_build.build_runners.dbt.runner", # _probe_dbt_engine + # ``_walk_contracts`` takes the contract filename from ``cli.artifact_paths``, which + # ``workspace_config`` above already imports. + "fluid_build.util.upstream_discovery", # _walk_contracts (directories to skip) ) # Repeat-user threshold — at this many forges we stop showing the panel. _REPEAT_USER_THRESHOLD = 5 # File extensions we recognise as candidate sample data. _SAMPLE_EXTENSIONS = (".csv", ".parquet", ".json", ".jsonl", ".tsv", ".duckdb") +# The specialization probe tallies the productType of the first contracts under the +# start directory. Its tree walk (``_find_contracts``) is bounded three ways: contracts +# taken, directory levels below the start, and directory entries read. At the caps an +# ordinary tree costs ~0.1s (a pathological one, 10,000 one-entry directories, ~0.6s of +# CPU), and the scan's stop flag ends the walk early either way. +_SPECIALIZATION_SAMPLE = 8 +_SPECIALIZATION_MAX_DEPTH = 6 +_SPECIALIZATION_MAX_ENTRIES = 10_000 +# One specialization walk at a time in the process. A walk the scan gave up on ends at +# its next directory once the scan's stop flag is set, but one stuck in a filesystem +# call (a hung network mount) never does, and each later scan would add another. Behind +# this lock the later walks wait and leave when their own scan returns (a caller with no +# scan gives up after one scan budget), so a process holds at most one stuck walker. +_WALK_GATE = threading.Lock() +# How often a walk waiting for the gate looks at its scan's stop flag. +_WALK_GATE_POLL_S = 0.05 # CLI binaries we report on. _CLI_BINARIES = ( "dbt", @@ -361,24 +383,99 @@ def _probe_return_user() -> Dict[str, Any]: return {"return_user": False, "forge_count": 0} -def _probe_specialization_suggestion(start: Path) -> Dict[str, Any]: - """Suggest a productType based on recent forge history. +def _walk_contracts(start: Path, stop: Optional[threading.Event]) -> List[Path]: + """The bounded walk behind :func:`_find_contracts`: depth-first, entries in name order.""" + from fluid_build.cli.artifact_paths import CONTRACT_FILENAME + from fluid_build.util.upstream_discovery import BUILD_OUTPUT_DIRS, IGNORED_DIRS - Reads the last 5 ``.fluid/forge-receipt.json`` entries under the - workspace and proposes the dominant type. Only fires when ≥4/5 - are the same type. + skip = IGNORED_DIRS | BUILD_OUTPUT_DIRS + found: List[Path] = [] + unread = _SPECIALIZATION_MAX_ENTRIES + + def visit(directory: str, depth: int) -> bool: + """Read ``directory``, then its subdirectories; True once the whole walk is over.""" + nonlocal unread + if unread <= 0 or (stop is not None and stop.is_set()): + return True + try: + with os.scandir(directory) as listing: + entries = list(islice(listing, unread)) + except OSError: + return False + unread -= len(entries) + # Names compare as path components do, so a contract file falls among the + # subdirectories exactly where ``sorted(rglob(...))`` would put it. + for entry in sorted(entries, key=lambda e: e.name): + try: + if entry.is_dir(follow_symlinks=False): + if depth < _SPECIALIZATION_MAX_DEPTH and entry.name not in skip: + if visit(entry.path, depth + 1): + return True + elif entry.name == CONTRACT_FILENAME and entry.is_file(follow_symlinks=False): + found.append(Path(entry.path)) + if len(found) >= _SPECIALIZATION_SAMPLE: + return True + except OSError: + continue + return False + + visit(str(start), 0) + return found + + +def _find_contracts(start: Path, stop: Optional[threading.Event] = None) -> List[Path]: + """The first ``_SPECIALIZATION_SAMPLE`` contracts under ``start``, in ``rglob`` order. + + Depth-first with each directory's entries in name order, which lists the contracts + exactly as ``sorted(start.rglob("contract.fluid.yaml"))`` does, so the walk can end + at the last one it needs instead of reading the whole tree first. It also ends on + its own bounds, and only the contracts it reached are returned: + + * directories on the repo's VCS / dependency / build-output list + (``upstream_discovery.IGNORED_DIRS`` and ``BUILD_OUTPUT_DIRS``: ``.git``, + ``.venv``, ``site-packages``, ``node_modules``, ``dist``, ...) are not entered; + * nothing deeper than ``_SPECIALIZATION_MAX_DEPTH`` levels below ``start`` is + listed, and at most ``_SPECIALIZATION_MAX_ENTRIES`` entries are read in all (a + directory larger than what is left of that is read in part); + * symlinks are neither followed nor tallied, and only a regular file counts as a + contract: ``rglob`` opens a symlink to ``/dev/zero`` or a FIFO named like one; + * ``stop`` is looked at before every directory. Python cannot kill a thread, so + this is how a scan ends the walk it abandoned. + + One walk runs at a time per process (``_WALK_GATE``); a walk that is made to wait + leaves with ``[]`` as soon as its ``stop`` is set, or after one scan budget if it + has none. + """ + deadline = time.monotonic() + _TOTAL_TIMEOUT_S + while not _WALK_GATE.acquire(timeout=_WALK_GATE_POLL_S): + if (stop is not None and stop.is_set()) or time.monotonic() >= deadline: + return [] + try: + return _walk_contracts(start, stop) + finally: + _WALK_GATE.release() + + +def _probe_specialization_suggestion( + start: Path, stop: Optional[threading.Event] = None +) -> Dict[str, Any]: + """Suggest a productType from the ``contract.fluid.yaml`` files under ``start``. + + Tallies ``metadata.productType`` over the first ``_SPECIALIZATION_SAMPLE`` + contracts ``_find_contracts`` returns and proposes the dominant type when at + least 4 of at least 5 typed contracts agree. ``stop`` is the scan's flag for a + probe it has given up on: the walk and the parse loop both end on it. """ try: - # Cheapest possible: glob for nearby contracts and tally their - # productType. Limited to a small radius so the scan stays - # under budget. - contracts = sorted(start.rglob("contract.fluid.yaml"))[:8] + contracts = _find_contracts(start, stop) if not contracts: return {} import yaml as _yaml types: List[str] = [] for c in contracts: + if stop is not None and stop.is_set(): + return {} try: doc = _yaml.safe_load(c.read_text(encoding="utf-8")) or {} pt = (doc.get("metadata") or {}).get("productType") @@ -456,7 +553,9 @@ def run_welcome_scan(*, start: Optional[Path] = None) -> WelcomeFindings: the dbt subprocess included); the probes then get ``_TOTAL_TIMEOUT_S`` of wall-clock. A probe still running at the deadline is dropped — its fields stay at their defaults — and abandoned, so a hung probe delays neither the - prompt nor the process exit. + prompt nor the process exit. The probe that walks the directory tree is told + to stop when the scan returns and ends at its next directory; the others run + on to their own end. Wall-clock is the cold imports plus at most ``_TOTAL_TIMEOUT_S``; ``scan_duration_ms`` reports all of it. @@ -464,6 +563,7 @@ def run_welcome_scan(*, start: Optional[Path] = None) -> WelcomeFindings: target = (start or Path.cwd()).resolve() findings = WelcomeFindings() started = time.monotonic() + stop = threading.Event() # set when the scan returns, see ``_find_contracts`` probes: Dict[str, Tuple[Callable[..., Dict[str, Any]], Tuple[Any, ...]]] = { "workspace": (_probe_workspace, (target,)), @@ -474,12 +574,15 @@ def run_welcome_scan(*, start: Optional[Path] = None) -> WelcomeFindings: "samples": (_probe_sample_data, (target,)), "cloud": (_probe_cloud_creds, ()), "return_user": (_probe_return_user, ()), - "specialization": (_probe_specialization_suggestion, (target,)), + "specialization": (_probe_specialization_suggestion, (target, stop)), } - futures = {name: _spawn_probe(name, fn, args) for name, (fn, args) in probes.items()} - _warm_probe_imports() - done, _ = wait(futures.values(), timeout=_TOTAL_TIMEOUT_S) + try: + futures = {name: _spawn_probe(name, fn, args) for name, (fn, args) in probes.items()} + _warm_probe_imports() + done, _ = wait(futures.values(), timeout=_TOTAL_TIMEOUT_S) + finally: + stop.set() for name, future in futures.items(): if future not in done: LOG.debug("welcome_scan_probe_timeout: %s", name) diff --git a/fluid_build/credentials/litellm_endpoints.py b/fluid_build/credentials/litellm_endpoints.py index 53d71d680..598d7de27 100644 --- a/fluid_build/credentials/litellm_endpoints.py +++ b/fluid_build/credentials/litellm_endpoints.py @@ -30,7 +30,8 @@ * ``_API_BASE`` for every ``litellm.provider_list`` entry (``-`` read as ``_``); -* every ``*_API_BASE`` name its provider code reads; +* every ``*_API_BASE`` name its provider code reads, with a kind after it + too (``DASHSCOPE_API_BASE_VIDEO``, ``QWENCLOUD_API_BASE_RERANK``); * every name its provider code reads through ``get_secret``, ``get_secret_str``, ``get_secret_bool``, ``os.getenv`` or ``os.environ`` that ends in ``_URL``, ``_ENDPOINT``, ``_HOST``, ``_BASE``, ``_ACCOUNT_ID`` @@ -110,6 +111,9 @@ "CUSTOM_OPENAI_API_BASE", "DARKBLOOM_API_BASE", "DASHSCOPE_API_BASE", + "DASHSCOPE_API_BASE_IMAGE", + "DASHSCOPE_API_BASE_RERANK", + "DASHSCOPE_API_BASE_VIDEO", "DATABRICKS_AGENT_API_BASE", "DATABRICKS_API_BASE", "DATAFORSEO_API_BASE", @@ -220,7 +224,13 @@ "PRISM_API_BASE", "PUBLICAI_API_BASE", "QWENCLOUD_API_BASE", + "QWENCLOUD_API_BASE_IMAGE", + "QWENCLOUD_API_BASE_RERANK", + "QWENCLOUD_API_BASE_VIDEO", "QWEN_AI_PLATFORM_API_BASE", + "QWEN_AI_PLATFORM_API_BASE_IMAGE", + "QWEN_AI_PLATFORM_API_BASE_RERANK", + "QWEN_AI_PLATFORM_API_BASE_VIDEO", "RAGFLOW_API_BASE", "RECRAFT_API_BASE", "REDUCTO_API_BASE", diff --git a/fluid_build/credentials/protected_env.py b/fluid_build/credentials/protected_env.py index 9c425457b..126c8e395 100644 --- a/fluid_build/credentials/protected_env.py +++ b/fluid_build/credentials/protected_env.py @@ -1256,9 +1256,18 @@ class EndpointGroup: EndpointGroup( service="Databricks model serving", endpoints=("DATABRICKS_API_BASE",), - credentials=("DATABRICKS_API_KEY", "DATABRICKS_TOKEN", _LLM_KEY), + credentials=( + "DATABRICKS_API_KEY", + "DATABRICKS_TOKEN", + # litellm's OAuth M2M path posts it to the workspace this base names + "DATABRICKS_CLIENT_SECRET", + _LLM_KEY, + ), ambient=True, - read_by=f"litellm's databricks/ route (DATABRICKS_API_BASE); {_LLM_KEYRING}", + read_by=( + "litellm's databricks/ route (DATABRICKS_API_BASE, and its OAuth M2M " + f"token request with DATABRICKS_CLIENT_SECRET); {_LLM_KEYRING}" + ), paired_by=((_LLM_KEY,),), ), EndpointGroup( diff --git a/tests/build_runners/test_catalog_auth_redaction.py b/tests/build_runners/test_catalog_auth_redaction.py index 1b721cd73..537f44eb8 100644 --- a/tests/build_runners/test_catalog_auth_redaction.py +++ b/tests/build_runners/test_catalog_auth_redaction.py @@ -80,7 +80,12 @@ def test_an_override_warehouse_password_is_masked(): def test_the_embedded_preflight_registers_the_variable_value(monkeypatch): monkeypatch.setenv("FW_LK_CREDENTIAL", f"spark:{_PASSWORD}") - binding = _binding("lakekeeper", uri="http://c:8181/catalog", warehouse="analytics") + # The operator pairs the variable with both hosts it reaches. + monkeypatch.setenv( + "FLUID_ACQUISITION_SECRET_REF_ALLOWLIST", "FW_LK_CREDENTIAL@c,FW_LK_CREDENTIAL@kc.example" + ) + monkeypatch.setenv("FLUID_ACQUISITION_DEPLOYMENT_HOST_ALLOWLIST", "c,kc.example") + binding = _binding("lakekeeper", uri="https://c:8443/catalog", warehouse="analytics") contract = _contract( binding, debezium={"deployment": {"mode": "embedded"}, "server": {"sink": {}}} ) diff --git a/tests/build_runners/test_catalog_auth_sink.py b/tests/build_runners/test_catalog_auth_sink.py index 57a6d1e03..02e8c36be 100644 --- a/tests/build_runners/test_catalog_auth_sink.py +++ b/tests/build_runners/test_catalog_auth_sink.py @@ -25,12 +25,15 @@ from __future__ import annotations import logging +import os import subprocess from pathlib import Path -from typing import Any, Dict, List +from typing import Any, Dict, List, Optional import pytest +from fluid_build.build_runners._deployment_host_policy import ALLOWLIST_ENV as HOST_ALLOWLIST +from fluid_build.build_runners._secret_policy import ALLOWLIST_ENV as SECRET_REF_ALLOWLIST from fluid_build.build_runners.debezium.iceberg_sink import emit_debezium_iceberg_sink_config from fluid_build.build_runners.kafka_connect.iceberg_sink import emit_iceberg_sink_config from fluid_build.providers._iceberg_catalog import ResolvedIcebergCatalog @@ -38,7 +41,22 @@ pytestmark = [pytest.mark.unit] -_KEYCLOAK = "http://keycloak:8080/realms/iceberg/protocol/openid-connect/token" + +@pytest.fixture(autouse=True) +def _operator_allows_the_fixture_hosts(request, monkeypatch): + """The operator pairs each fixture variable with each fixture host. Where a + credential may go is pinned in test_iceberg_sink_catalog_destinations.py.""" + if "kafka_connect_mock" in request.fixturenames: + # The mock may set the host list for its own server: extend it after. + request.getfixturevalue("kafka_connect_mock") + hosts = ("lakekeeper", "keycloak") + names = ("LAKEKEEPER_CREDENTIAL", "UC_TOKEN") + monkeypatch.setenv(SECRET_REF_ALLOWLIST, ",".join(f"{n}@{h}" for n in names for h in hosts)) + current = [h for h in os.environ.get(HOST_ALLOWLIST, "").split(",") if h.strip()] + monkeypatch.setenv(HOST_ALLOWLIST, ",".join([*current, *hosts])) + + +_KEYCLOAK = "https://keycloak:8443/realms/iceberg/protocol/openid-connect/token" _SECRET_VALUE = "spark:2OR3eRvYfSZzzZ16MlPd95jhLnOaLM52" # pragma: allowlist secret _OAUTH2 = { "type": "oauth2", @@ -52,7 +70,7 @@ catalog_type="rest", warehouse="demo", fq_table="bronze.orders", - uri="http://lakekeeper:8181/catalog", + uri="https://lakekeeper:8443/catalog", kind="lakekeeper", ) @@ -118,7 +136,12 @@ def test_emitters_are_deterministic_with_auth(): # ── Kafka Connect runner ──────────────────────────────────────────────────── -def _kc_contract(catalog_auth: Dict[str, Any], **kc: Any) -> Dict[str, Any]: +def _kc_contract( + catalog_auth: Optional[Dict[str, Any]], *, location: Optional[Dict[str, Any]] = None, **kc: Any +) -> Dict[str, Any]: + sink: Dict[str, Any] = {"format": "iceberg"} + if catalog_auth is not None: + sink["catalogAuth"] = catalog_auth return { "fluidVersion": "0.7.6", "kind": "DataProduct", @@ -137,7 +160,7 @@ def _kc_contract(catalog_auth: Dict[str, Any], **kc: Any) -> Dict[str, Any]: "mode": "incremental_append", "streams": ["public.orders"], }, - "sink": {"format": "iceberg", "catalogAuth": catalog_auth}, + "sink": sink, "kafka-connect": { "deployment": {"server_url": "http://kafka-connect.test:8083"}, "connector_name": "src", @@ -154,13 +177,17 @@ def _kc_contract(catalog_auth: Dict[str, Any], **kc: Any) -> Dict[str, Any]: "binding": { "platform": "local", "format": "iceberg", - "location": { - "catalog": "lakekeeper", - "uri": "http://lakekeeper:8181/catalog", - "warehouse": "demo", - "database": "bronze", - "table": "orders", - }, + "location": ( + dict(location) + if location is not None + else { + "catalog": "lakekeeper", + "uri": "https://lakekeeper:8443/catalog", + "warehouse": "demo", + "database": "bronze", + "table": "orders", + } + ), }, "contract": {"schema": []}, } @@ -216,8 +243,13 @@ def test_kafka_connect_runner_refuses_an_override_that_sets_auth( # ── embedded Debezium Server runner ───────────────────────────────────────── -def _dbz_contract(catalog_auth: Dict[str, Any], server_sink: Any = None) -> Dict[str, Any]: - contract = _kc_contract(catalog_auth) +def _dbz_contract( + catalog_auth: Optional[Dict[str, Any]], + server_sink: Any = None, + *, + location: Optional[Dict[str, Any]] = None, +) -> Dict[str, Any]: + contract = _kc_contract(catalog_auth, location=location) contract["id"] = "bronze.dbz_auth" build = contract["builds"][0] build["engine"] = "debezium" @@ -251,8 +283,8 @@ def __call__(self, argv: List[str], **kwargs: Any) -> subprocess.CompletedProces return subprocess.CompletedProcess(argv, 0, stdout="", stderr="") -@pytest.fixture -def fake_server(monkeypatch): +def _install_fake_server(monkeypatch) -> _RecordingRun: + """Fake the Debezium Server binary and its start; returns the recorder.""" from fluid_build.build_runners.debezium import runner as dbz_runner recorder = _RecordingRun() @@ -261,6 +293,11 @@ def fake_server(monkeypatch): return recorder +@pytest.fixture +def fake_server(monkeypatch): + return _install_fake_server(monkeypatch) + + def test_embedded_writes_env_references_and_the_server_inherits_the_environment( tmp_path: Path, monkeypatch, fake_server, caplog ): diff --git a/tests/build_runners/test_catalog_auth_validation.py b/tests/build_runners/test_catalog_auth_validation.py index aa77af3b0..88d9e8c8c 100644 --- a/tests/build_runners/test_catalog_auth_validation.py +++ b/tests/build_runners/test_catalog_auth_validation.py @@ -23,10 +23,13 @@ from __future__ import annotations +import os from typing import Any, Dict, Optional import pytest +from fluid_build.build_runners._deployment_host_policy import ALLOWLIST_ENV as HOST_ALLOWLIST +from fluid_build.build_runners._secret_policy import ALLOWLIST_ENV as SECRET_REF_ALLOWLIST from fluid_build.build_runners.kafka_connect.iceberg_sink_validation import ( iceberg_sink_preflight, validate_iceberg_sink, @@ -35,7 +38,19 @@ pytestmark = [pytest.mark.unit] -_KEYCLOAK = "http://keycloak:8080/realms/iceberg/protocol/openid-connect/token" + +@pytest.fixture(autouse=True) +def _operator_allows_the_fixture_hosts(monkeypatch): + """The operator pairs each fixture variable with each fixture host. Where a + credential may go is pinned in test_iceberg_sink_catalog_destinations.py.""" + hosts = ("lakekeeper", "keycloak", "catalog", "c") + names = ("LAKEKEEPER_CREDENTIAL", "UC_TOKEN", "LK_CRED") + monkeypatch.setenv(SECRET_REF_ALLOWLIST, ",".join(f"{n}@{h}" for n in names for h in hosts)) + current = [h for h in os.environ.get(HOST_ALLOWLIST, "").split(",") if h.strip()] + monkeypatch.setenv(HOST_ALLOWLIST, ",".join([*current, *hosts])) + + +_KEYCLOAK = "https://keycloak:8443/realms/iceberg/protocol/openid-connect/token" _OAUTH2 = { "type": "oauth2", "credentialEnv": "LAKEKEEPER_CREDENTIAL", @@ -44,7 +59,7 @@ } _LAKEKEEPER_LOCATION = { "catalog": "lakekeeper", - "uri": "http://lakekeeper:8181/catalog", + "uri": "https://lakekeeper:8443/catalog", "warehouse": "demo", "database": "bronze", "table": "orders", @@ -114,7 +129,7 @@ def _dbz(catalog_auth: Optional[Dict[str, Any]], *, server_sink=None, mode="embe def test_every_rest_kind_accepts_catalog_auth(catalog): location = {**_LAKEKEEPER_LOCATION, "catalog": catalog} if catalog != "lakekeeper": - location["uri"] = "http://catalog:8181/api/catalog" + location["uri"] = "https://catalog:8443/api/catalog" errors, _warnings = validate_iceberg_sink(_kc(dict(_OAUTH2), location=location)) assert errors == [] @@ -194,7 +209,7 @@ def test_a_handwritten_sink_config_is_refused(contract, where): assert f"{prefix}rest.auth.type to oauth2" in hit[0] # A contract value is named by its field, not echoed (it could hold a secret). assert f"{prefix}oauth2-server-uri to " in hit[0] - assert "keycloak:8080" not in hit[0] + assert "keycloak:8443" not in hit[0] def test_a_handwritten_config_with_a_userinfo_uri_is_refused_without_echoing_it(): @@ -265,7 +280,7 @@ def test_debezium_override_of_an_auth_property_is_refused(key): def test_an_unrelated_override_key_is_not_a_conflict(): kc = { "iceberg_catalog_overrides": { - "iceberg.catalog.uri": "http://c:8181/catalog", + "iceberg.catalog.uri": "https://c:8443/catalog", "s3.region": "y", } } diff --git a/tests/build_runners/test_iceberg_sink_catalog_destinations.py b/tests/build_runners/test_iceberg_sink_catalog_destinations.py new file mode 100644 index 000000000..c0ccf2783 --- /dev/null +++ b/tests/build_runners/test_iceberg_sink_catalog_destinations.py @@ -0,0 +1,675 @@ +# Copyright 2024-2026 Agentics Transformation Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Where a ``sink.catalogAuth`` credential may go, and what location text may say. + +A contract names the variable (``tokenEnv`` / ``credentialEnv``) and every host +it is sent to (``binding.location.uri``, ``oauth2ServerUri``). The Kafka Connect +worker or the embedded Debezium Server resolves ``env:`` from its own +environment, which for Debezium is this runner's whole environment, and sends +the value there. So the operator pairs the variable with each host +(``NAME@host``), a hostname needs a host-list entry (the remote JVM resolves it +itself, later), an IP literal keeps the deployment host rules, loopback is +exempt, and the credential travels over https only. Text from +``binding.location`` that reaches the derived sink config may not reference a +variable itself (``env:`` / ``${``) or hold a control character. + +Every refusal happens before any Connect REST call, ``application.properties`` +write or read of the variable. +""" + +from __future__ import annotations + +import os +from pathlib import Path +from typing import Any, Dict, List, Optional + +import pytest + +from fluid_build.build_runners._deployment_host_policy import ALLOWLIST_ENV as HOST_ALLOWLIST +from fluid_build.build_runners._secret_policy import ALLOWLIST_ENV as SECRET_REF_ALLOWLIST +from fluid_build.build_runners.kafka_connect.iceberg_sink_validation import ( + iceberg_sink_preflight, + validate_iceberg_sink, +) +from tests.build_runners import test_catalog_auth_sink as _sink_tests +from tests.build_runners.test_catalog_auth_sink import _RecordingRun + +pytestmark = [pytest.mark.unit] + +_SECRET = "OPERATOR_SYNTH_SECRET" # pragma: allowlist secret - a variable NAME +_SECRET_VALUE = "operator-synth-value-7Hq2" # pragma: allowlist secret +#: AWS's documented example secret access key: the shape a rendered secret takes. +_AWS_DOC_KEY = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" # pragma: allowlist secret +_CATALOG_HOST = "catalog.attacker.example" +_IDP_HOST = "idp.attacker.example" +_CATALOG = f"https://{_CATALOG_HOST}/catalog" +_IDP = f"https://{_IDP_HOST}/realms/iceberg/protocol/openid-connect/token" + +_AUTHS: Dict[str, Dict[str, str]] = { + "bearer": {"type": "bearer", "tokenEnv": _SECRET}, + "oauth2": { + "type": "oauth2", + "credentialEnv": _SECRET, + "oauth2ServerUri": _IDP, + "scope": "lakekeeper", + }, +} +#: The property each type's secret reference lands in. +_SECRET_PROPERTY = {"bearer": "token", "oauth2": "credential"} + + +def _hosts(auth: str) -> List[str]: + """Every host the ``auth`` credential reaches: the catalog, and an OAuth2 IdP.""" + return [_CATALOG_HOST, _IDP_HOST] if auth == "oauth2" else [_CATALOG_HOST] + + +@pytest.fixture(autouse=True) +def _operator_environment(monkeypatch): + """No operator entry unless a test adds one, whatever the shell exports. The + variable is set, so the embedded Debezium preflight's own "set?" check + passes. Autouse runs first, so a mock fixture's own host entry survives.""" + monkeypatch.delenv(SECRET_REF_ALLOWLIST, raising=False) + monkeypatch.delenv(HOST_ALLOWLIST, raising=False) + monkeypatch.setenv(_SECRET, _SECRET_VALUE) + + +def _operator_allows(monkeypatch, *, pairs=(), hosts=()) -> None: + """Set the operator's lists. Host entries are ADDED: a fixture may list its + own synthetic Connect host there.""" + if pairs: + monkeypatch.setenv(SECRET_REF_ALLOWLIST, ",".join(pairs)) + if hosts: + current = [h for h in os.environ.get(HOST_ALLOWLIST, "").split(",") if h.strip()] + monkeypatch.setenv(HOST_ALLOWLIST, ",".join([*current, *hosts])) + + +def _allow_everything(monkeypatch, auth: str) -> None: + hosts = _hosts(auth) + _operator_allows(monkeypatch, pairs=[f"{_SECRET}@{h}" for h in hosts], hosts=hosts) + + +def _location(uri: str = _CATALOG, **extra: Any) -> Dict[str, Any]: + return { + "catalog": "lakekeeper", + "uri": uri, + "warehouse": "demo", + "database": "bronze", + "table": "orders", + **extra, + } + + +def _kc_contract( + catalog_auth: Optional[Dict[str, Any]] = None, + *, + location: Optional[Dict[str, Any]] = None, + **kc: Any, +) -> Dict[str, Any]: + """The catalogAuth sink tests' Kafka Connect contract, aimed at a remote catalog.""" + return _sink_tests._kc_contract( + catalog_auth, location=location if location is not None else _location(), **kc + ) + + +def _dbz_contract( + catalog_auth: Optional[Dict[str, Any]] = None, + *, + location: Optional[Dict[str, Any]] = None, + server_sink: Optional[Dict[str, Any]] = None, +) -> Dict[str, Any]: + """The catalogAuth sink tests' embedded Debezium contract, aimed at a remote catalog.""" + return _sink_tests._dbz_contract( + catalog_auth, server_sink, location=location if location is not None else _location() + ) + + +@pytest.fixture +def fake_server(monkeypatch) -> _RecordingRun: + return _sink_tests._install_fake_server(monkeypatch) + + +class _Engine: + """One streaming runner: builds its contract, runs it, reads what it delivered.""" + + def __init__(self, name: str, kc_mock: Any, server: _RecordingRun, tmp_path: Path) -> None: + self.name = name + self._kc_mock = kc_mock + self._server = server + self._tmp_path = tmp_path + + def contract(self, catalog_auth=None, **kwargs: Any) -> Dict[str, Any]: + if self.name == "kafka-connect": + return _kc_contract(catalog_auth, **kwargs) + return _dbz_contract(catalog_auth, **kwargs) + + def run(self, contract: Dict[str, Any]) -> int: + if self.name == "kafka-connect": + from fluid_build.build_runners.kafka_connect.runner import execute_kafka_connect_build + + return execute_kafka_connect_build(contract["builds"][0], contract, self._tmp_path) + from fluid_build.build_runners.debezium.runner import execute_debezium_build + + return execute_debezium_build(contract["builds"][0], contract, self._tmp_path) + + def _properties(self, contract: Dict[str, Any]) -> Path: + return ( + self._tmp_path + / ".fluid" + / "debezium" + / contract["id"] + / "ingest" + / "application.properties" + ) + + def touched(self, contract: Dict[str, Any]) -> bool: + """Did the run reach Connect, write application.properties or start the server?""" + if self.name == "kafka-connect": + return bool(self._kc_mock.calls) + return self._properties(contract).exists() or bool(self._server.calls) + + def delivered(self, contract: Dict[str, Any]) -> Dict[str, str]: + """The catalog properties the run pushed or wrote, as bare keys.""" + if self.name == "kafka-connect": + cfg = self._kc_mock.connectors["src-sink"]["config"] + prefix = "iceberg.catalog." + else: + lines = self._properties(contract).read_text().splitlines() + cfg = dict(line.split("=", 1) for line in lines if "=" in line) + prefix = "debezium.sink.iceberg." + return {k[len(prefix) :]: v for k, v in cfg.items() if k.startswith(prefix)} + + +@pytest.fixture(params=["kafka-connect", "debezium"]) +def engine(request, kafka_connect_mock, fake_server, tmp_path) -> _Engine: + return _Engine(request.param, kafka_connect_mock, fake_server, tmp_path) + + +# ── the confirmed defect: a contract-chosen variable to a contract-chosen host ── + + +@pytest.mark.parametrize("auth", sorted(_AUTHS)) +def test_an_unpaired_variable_is_refused_before_anything_is_sent(engine, auth): + contract = engine.contract(dict(_AUTHS[auth])) + assert engine.run(contract) == 1 + assert not engine.touched(contract) + + +@pytest.mark.parametrize("auth", sorted(_AUTHS)) +def test_listed_pairs_and_hosts_reach_the_sink(engine, auth, monkeypatch): + _allow_everything(monkeypatch, auth) + contract = engine.contract(dict(_AUTHS[auth])) + assert engine.run(contract) == 0 + delivered = engine.delivered(contract) + assert delivered[_SECRET_PROPERTY[auth]] == f"env:{_SECRET}" + assert delivered["uri"] == _CATALOG + + +@pytest.mark.parametrize("engine_name", ["kafka-connect", "debezium"]) +def test_oauth2_pairs_the_variable_with_the_token_endpoint_too(engine_name, monkeypatch): + """The IdP receives the credential itself, the catalog a token minted from it.""" + _operator_allows( + monkeypatch, pairs=[f"{_SECRET}@{_CATALOG_HOST}"], hosts=[_CATALOG_HOST, _IDP_HOST] + ) + build = _kc_contract if engine_name == "kafka-connect" else _dbz_contract + error = iceberg_sink_preflight(build(dict(_AUTHS["oauth2"])), "ingest") + assert error is not None + assert f'{SECRET_REF_ALLOWLIST}="{_SECRET}@{_CATALOG_HOST},{_SECRET}@{_IDP_HOST}"' in error + assert "OAuth2 token endpoint" in error + + +@pytest.mark.parametrize("auth", sorted(_AUTHS)) +def test_a_paired_hostname_still_needs_a_host_entry(auth, monkeypatch): + """The remote JVM resolves the name itself, later: only the operator can vouch for it.""" + _operator_allows(monkeypatch, pairs=[f"{_SECRET}@{h}" for h in _hosts(auth)]) + error = iceberg_sink_preflight(_kc_contract(dict(_AUTHS[auth])), "ingest") + assert error is not None + assert f'{HOST_ALLOWLIST}="' in error + assert _CATALOG_HOST in error + + +def test_a_bare_name_entry_does_not_open_every_host(monkeypatch): + """``NAME`` (any host) still leaves the destination to the host list.""" + _operator_allows(monkeypatch, pairs=[_SECRET]) + assert iceberg_sink_preflight(_kc_contract(dict(_AUTHS["bearer"])), "ingest") is not None + _operator_allows(monkeypatch, hosts=[_CATALOG_HOST]) + assert iceberg_sink_preflight(_kc_contract(dict(_AUTHS["bearer"])), "ingest") is None + + +def test_a_parent_domain_entry_covers_the_catalog_host(monkeypatch): + _operator_allows(monkeypatch, pairs=[f"{_SECRET}@{_CATALOG_HOST}"], hosts=["attacker.example"]) + assert iceberg_sink_preflight(_kc_contract(dict(_AUTHS["bearer"])), "ingest") is None + + +@pytest.mark.parametrize("auth", sorted(_AUTHS)) +def test_a_loopback_catalog_needs_no_entry(engine, auth): + raw = dict(_AUTHS[auth]) + if auth == "oauth2": + raw["oauth2ServerUri"] = ( + "https://127.0.0.1:8443/realms/iceberg/protocol/openid-connect/token" + ) + contract = engine.contract(raw, location=_location("https://localhost:8443/catalog")) + assert engine.run(contract) == 0 + assert engine.delivered(contract)[_SECRET_PROPERTY[auth]] == f"env:{_SECRET}" + + +def test_validate_warns_where_the_run_refuses(): + """``fluid validate`` runs where the operator's lists may not be; the run is where.""" + contract = _kc_contract(dict(_AUTHS["bearer"])) + errors, warnings = validate_iceberg_sink(contract) + assert errors == [] + assert any(f"{_SECRET}@{_CATALOG_HOST}" in w for w in warnings) + error = iceberg_sink_preflight(contract, "ingest") + assert error is not None + assert error.startswith("iceberg sink preflight failed: ") # run-only, no "see validate" + # One export line sets both, ready to paste. + assert ( + f'export {SECRET_REF_ALLOWLIST}="{_SECRET}@{_CATALOG_HOST}" ' f'{HOST_ALLOWLIST}="' in error + ) + assert _SECRET_VALUE not in error + + +def test_the_suggestion_keeps_the_operators_entries(monkeypatch): + _operator_allows(monkeypatch, pairs=["OTHER@db.example.com"], hosts=["db.example.com"]) + error = iceberg_sink_preflight(_kc_contract(dict(_AUTHS["bearer"])), "ingest") + assert error is not None + assert f'{SECRET_REF_ALLOWLIST}="OTHER@db.example.com,{_SECRET}@{_CATALOG_HOST}"' in error + assert f'{HOST_ALLOWLIST}="db.example.com,{_CATALOG_HOST}"' in error + + +def test_a_refused_variable_is_never_read(monkeypatch, fake_server, tmp_path): + """The embedded preflight reads the variable to test it is set; not when refused.""" + from fluid_build.build_runners.debezium.runner import execute_debezium_build + + registered: List[Any] = [] + monkeypatch.setattr( + "fluid_build.observability.secret_redactor.register_secret", registered.append + ) + contract = _dbz_contract(dict(_AUTHS["bearer"])) + assert execute_debezium_build(contract["builds"][0], contract, tmp_path) == 1 + assert registered == [] + + +def test_the_pairing_follows_an_override_uri(monkeypatch): + """The credential goes to the uri that reaches the sink, not to the binding's.""" + _allow_everything(monkeypatch, "bearer") + kc = _kc_contract( + dict(_AUTHS["bearer"]), + iceberg_catalog_overrides={"iceberg.catalog.uri": "https://other.attacker.example/c"}, + ) + error = iceberg_sink_preflight(kc, "ingest") + assert error is not None and "other.attacker.example" in error + dbz = _dbz_contract( + dict(_AUTHS["bearer"]), + server_sink={ + "type": "iceberg", + "iceberg_sink_enabled": True, + "config": {"uri": "https://other.attacker.example/c"}, + }, + ) + error = iceberg_sink_preflight(dbz, "ingest") + assert error is not None and "other.attacker.example" in error + + +# ── https ─────────────────────────────────────────────────────────────────── + + +@pytest.mark.parametrize( + "auth, field", + [("bearer", "uri"), ("oauth2", "uri"), ("oauth2", "oauth2ServerUri")], +) +@pytest.mark.parametrize("engine_name", ["kafka-connect", "debezium"]) +def test_a_credential_travels_over_https_only(auth, field, engine_name): + """Loopback included, as on the managed Flink lane: refused by ``fluid validate``.""" + raw = dict(_AUTHS[auth]) + location = _location("https://localhost:8443/catalog") + if field == "uri": + location["uri"] = "http://localhost:8181/catalog" + else: + raw["oauth2ServerUri"] = "http://127.0.0.1:8080/realms/x/protocol/openid-connect/token" + build = _kc_contract if engine_name == "kafka-connect" else _dbz_contract + errors, _warnings = validate_iceberg_sink(build(raw, location=location)) + label = "sink.catalogAuth.oauth2ServerUri" if field != "uri" else "binding.location.uri" + assert any(label in e and "https://" in e for e in errors), errors + + +def test_an_unauthenticated_http_catalog_is_unchanged(): + contract = _kc_contract(None, location=_location("http://lakekeeper:8181/catalog")) + assert validate_iceberg_sink(contract) == ([], []) + assert iceberg_sink_preflight(contract, "ingest") is None + + +# ── IP literals keep the deployment host rules ───────────────────────────── + + +@pytest.mark.parametrize( + "host", ["169.254.169.254", "[fe80::1]", "0.0.0.0", "[::ffff:169.254.169.254]"] +) +def test_a_link_local_catalog_is_refused_even_when_listed(host, monkeypatch): + bare = host.strip("[]") + _operator_allows(monkeypatch, pairs=[_SECRET, f"{_SECRET}@{bare}"], hosts=[bare]) + contract = _kc_contract(dict(_AUTHS["bearer"]), location=_location(f"https://{host}/catalog")) + errors, _warnings = validate_iceberg_sink(contract) + assert any("link-local" in e for e in errors), errors + + +def test_a_public_ip_needs_a_host_entry(monkeypatch): + contract = _kc_contract( + dict(_AUTHS["bearer"]), location=_location("https://93.184.216.34/catalog") + ) + _operator_allows(monkeypatch, pairs=[f"{_SECRET}@93.184.216.34"]) + error = iceberg_sink_preflight(contract, "ingest") + assert error is not None and f'{HOST_ALLOWLIST}="' in error + _operator_allows(monkeypatch, hosts=["93.184.216.34"]) + assert iceberg_sink_preflight(contract, "ingest") is None + + +def test_a_private_ip_needs_only_the_pairing(monkeypatch): + contract = _kc_contract(dict(_AUTHS["bearer"]), location=_location("https://10.0.0.5/catalog")) + assert iceberg_sink_preflight(contract, "ingest") is not None + _operator_allows(monkeypatch, pairs=[f"{_SECRET}@10.0.0.5"]) + assert iceberg_sink_preflight(contract, "ingest") is None + + +# ── binding.location text in the derived sink config ─────────────────────── + + +_REFERENCE_TEXT = [ + ("region", f"env:{_SECRET}"), + ("warehouse", f"env:{_SECRET}"), + ("uri", f"env:{_SECRET}"), + ("warehouse", f"${{{_SECRET}}}"), + ("uri", f"https://${{{_SECRET}}}.attacker.example/catalog"), + ("database", f"${{env:{_SECRET}}}"), + ("warehouse", "demo\nx"), +] + + +@pytest.mark.parametrize("field, value", _REFERENCE_TEXT) +def test_location_text_may_not_reference_a_variable(engine, field, value): + contract = engine.contract(None, location=_location(**{field: value})) + errors, _warnings = validate_iceberg_sink(contract) + mine = [e for e in errors if f"binding.location.{field}" in e and "env:" in e] + assert mine, errors + assert not any(_SECRET in e for e in mine) # the value is not echoed + assert engine.run(contract) == 1 + assert not engine.touched(contract) + + +def test_kafka_connect_table_and_partition_text_is_checked_too(): + contract = _kc_contract(None, location=_location(table=f"${{env:{_SECRET}}}")) + assert any("binding.location.table" in e for e in validate_iceberg_sink(contract)[0]) + contract = _kc_contract(None, location=_location(partitionBy=[f"env:{_SECRET}"])) + assert any("binding.location.partitionBy" in e for e in validate_iceberg_sink(contract)[0]) + + +def test_bigquery_catalog_properties_are_checked(): + location = { + "catalog": "bigquery", + "project": f"env:{_SECRET}", + "dataset": "bronze", + "table": "orders", + "bucket": "lake", + } + contract = _kc_contract(None, location=location) + contract["exposes"][0]["binding"]["platform"] = "gcp" + errors, _warnings = validate_iceberg_sink(contract) + assert any("binding.location.project" in e for e in errors), errors + + +@pytest.mark.parametrize( + "region, accepted", + [ + ("eu-central-1", True), + ("us-gov-west-1", True), + ("westeurope", True), + ("EU", True), # a host label ignores case + ("attacker.example", False), + ("us-east-1/", False), + ("us-east-1#", False), + ("-eu", False), + ("a" * 64, False), + ], +) +@pytest.mark.parametrize("engine_name", ["kafka-connect", "debezium"]) +def test_client_region_is_a_host_label(region, accepted, engine_name): + """``client.region`` becomes part of the S3 and Glue endpoint hosts.""" + build = _kc_contract if engine_name == "kafka-connect" else _dbz_contract + errors, _warnings = validate_iceberg_sink(build(None, location=_location(region=region))) + refused = any("binding.location.region" in e for e in errors) + assert refused is not accepted, errors + + +def _rendered(contract: Dict[str, Any]) -> Dict[str, Any]: + """The contract as the acquisition dispatcher hands it to a runner.""" + from fluid_build.build_runners.base import _resolve_env_placeholders + + return _resolve_env_placeholders(contract) + + +@pytest.mark.parametrize( + "rendered_to, refused", + [("eu-central-1", False), ("attacker.example", True)], +) +def test_a_templated_region_is_checked_where_it_is_rendered(monkeypatch, rendered_to, refused): + """``fluid validate`` sees the template; the run's preflight sees the value.""" + contract = _kc_contract(None, location=_location(region="{{ env.LAKE_REGION }}")) + assert not any("binding.location.region" in e for e in validate_iceberg_sink(contract)[0]) + monkeypatch.setenv("LAKE_REGION", rendered_to) + error = iceberg_sink_preflight(_rendered(contract), "ingest") + assert (error is not None and "binding.location.region" in error) is refused + + +def test_a_templated_catalog_uri_is_checked_where_it_is_rendered(monkeypatch): + contract = _kc_contract(dict(_AUTHS["bearer"]), location=_location("{{ env.CATALOG_URI }}")) + errors, warnings = validate_iceberg_sink(contract) + assert errors == [] + assert not any(_SECRET in w for w in warnings) + monkeypatch.setenv("CATALOG_URI", _CATALOG) + error = iceberg_sink_preflight(_rendered(contract), "ingest") + assert error is not None and f"{_SECRET}@{_CATALOG_HOST}" in error + _allow_everything(monkeypatch, "bearer") + assert iceberg_sink_preflight(_rendered(contract), "ingest") is None + + +def test_a_handwritten_sink_config_reads_no_location_text(): + """Nothing is derived, so the binding's text never reaches the sink config.""" + contract = _kc_contract( + None, + location=_location(region="attacker.example"), + sink_connector_config={"connector.class": "x"}, + ) + assert not any("binding.location.region" in e for e in validate_iceberg_sink(contract)[0]) + + +# ── a host the refusal can repeat safely ─────────────────────────────────── + + +@pytest.mark.parametrize( + "uri", + [ + 'https://x"$(id)".attacker.example/catalog', + "https://x,OTHER_VAR.attacker.example/catalog", + "https://faß.example/catalog", + "https://[fd00::1%25eth0]/catalog", + ], + ids=["shell", "comma", "non-ascii", "zone-id"], +) +def test_a_host_that_is_not_plain_is_refused_and_never_echoed(uri, monkeypatch): + """The refusal's export line repeats the host, so it must be safe to paste.""" + _operator_allows(monkeypatch, pairs=[_SECRET], hosts=["attacker.example", "example"]) + contract = _kc_contract(dict(_AUTHS["bearer"]), location=_location(uri)) + errors, _warnings = validate_iceberg_sink(contract) + mine = [e for e in errors if "not a plain DNS name or IP address" in e] + assert mine, errors + error = iceberg_sink_preflight(contract, "ingest") + assert error is not None + for text in (*mine, error): + assert "export" not in text + assert "$(id)" not in text and "other_var" not in text.lower() + assert "faß" not in text and "eth0" not in text + + +@pytest.mark.parametrize( + "uri", + [ + "https://evil.example\\@127.0.0.1/catalog", + "https://u:p4ss@catalog.attacker.example/c", # pragma: allowlist secret + ], + ids=["backslash", "userinfo"], +) +def test_a_user_password_part_is_refused(uri): + """Python and the JVM read such an authority differently; it never reaches a check.""" + contract = _kc_contract(dict(_AUTHS["bearer"]), location=_location(uri)) + errors, _warnings = validate_iceberg_sink(contract) + mine = [e for e in errors if "user:password@ part" in e] + assert mine, errors + assert not any("p4ss" in e for e in mine) + + +def test_an_engine_that_derives_no_sink_config_skips_the_location_text(): + """Only the Kafka Connect and Debezium runners derive the config from the binding.""" + contract = _kc_contract(None, location=_location(warehouse=f"env:{_SECRET}")) + contract["builds"][0]["engine"] = "airbyte" + errors, _warnings = validate_iceberg_sink(contract) + assert not any("binding.location.warehouse" in e for e in errors), errors + + +def test_a_rendered_region_is_not_echoed(monkeypatch): + """At run a region may hold whatever a {{ env.* }} template rendered.""" + contract = _kc_contract(None, location=_location(region="{{ env.LAKE_REGION }}")) + monkeypatch.setenv("LAKE_REGION", _AWS_DOC_KEY) + error = iceberg_sink_preflight(_rendered(contract), "ingest") + assert error is not None and "binding.location.region" in error + assert _AWS_DOC_KEY[:13] not in error + + +def test_validate_defers_a_template_only_where_it_decides_the_host(): + """A template in the path leaves the scheme and host to check now.""" + raw = dict(_AUTHS["bearer"]) + contract = _kc_contract(raw, location=_location("http://169.254.169.254/{{ env.X }}")) + errors, _warnings = validate_iceberg_sink(contract) + assert any("must be an https:// URL" in e for e in errors), errors + + +def test_a_catalog_not_reached_over_rest_never_reads_the_variable(monkeypatch): + """catalogAuth on Glue is already refused; the embedded preflight reads nothing.""" + registered: List[Any] = [] + monkeypatch.setattr( + "fluid_build.observability.secret_redactor.register_secret", registered.append + ) + location = {"database": "bronze", "table": "orders", "bucket": "lake", "region": "us-east-1"} + contract = _dbz_contract(dict(_AUTHS["bearer"]), location=location) + contract["exposes"][0]["binding"]["platform"] = "aws" + error = iceberg_sink_preflight(contract, "ingest") + assert error is not None and "not reached over REST" in error + assert registered == [] + + +@pytest.mark.parametrize("engine_name", ["kafka-connect", "debezium"]) +def test_sink_partition_by_is_the_text_checked_when_set(engine_name): + """The runner's SinkSpec reads sink.partitionBy over the binding's.""" + build = _kc_contract if engine_name == "kafka-connect" else _dbz_contract + contract = build(None, location=_location(partitionBy=["day"])) + contract["builds"][0]["properties"]["sink"]["partitionBy"] = [f"${{env:{_SECRET}}}"] + errors, _warnings = validate_iceberg_sink(contract) + assert any("sink.partitionBy" in e for e in errors), errors + contract = build(None, location=_location(partitionBy=[f"env:{_SECRET}"])) + contract["builds"][0]["properties"]["sink"]["partitionBy"] = ["day"] + assert not any("partitionBy" in e for e in validate_iceberg_sink(contract)[0]) + + +def test_only_the_namespace_field_the_sink_reads_is_checked(): + """iceberg_namespace reads the first one set; the others reach nothing.""" + contract = _kc_contract(None, location=_location(dataset=f"${{{_SECRET}}}")) + assert not any("dataset" in e for e in validate_iceberg_sink(contract)[0]) + location = _location() + del location["database"] + contract = _kc_contract(None, location={**location, "dataset": f"${{{_SECRET}}}"}) + assert any("binding.location.dataset" in e for e in validate_iceberg_sink(contract)[0]) + + +def test_a_written_out_scheme_is_checked_while_the_host_waits_on_a_template(): + contract = _kc_contract(dict(_AUTHS["bearer"]), location=_location("http://{{ env.H }}/c")) + errors, _warnings = validate_iceberg_sink(contract) + assert any("must be an https:// URL" in e for e in errors), errors + + +def test_a_percent_suffix_does_not_make_a_name_an_address(monkeypatch): + """10.0.0.1%25.attacker.example is a name, and not a plain one.""" + _operator_allows(monkeypatch, pairs=[_SECRET]) + contract = _kc_contract( + dict(_AUTHS["bearer"]), location=_location("https://10.0.0.1%25.attacker.example/c") + ) + error = iceberg_sink_preflight(contract, "ingest") + assert error is not None and "not a plain DNS name or IP address" in error + + +def test_a_derived_warehouse_names_where_it_came_from(): + location = {"database": "bronze", "table": "orders", "bucket": "lake", "path": "p/${X}/"} + contract = _kc_contract(None, location=location) + contract["exposes"][0]["binding"]["platform"] = "aws" + errors, _warnings = validate_iceberg_sink(contract) + assert any("the warehouse derived from binding.location" in e for e in errors), errors + + +@pytest.mark.parametrize( + "uri", ["https:///catalog", "https://[::1/catalog"], ids=["empty", "bracket"] +) +def test_a_destination_with_no_readable_host_is_refused(uri): + contract = _kc_contract(dict(_AUTHS["bearer"]), location=_location(uri)) + errors, _warnings = validate_iceberg_sink(contract) + assert any("names no host" in e for e in errors), errors + + +@pytest.mark.parametrize( + "token_endpoint, refused", + [ + (_IDP, "a remote IdP beside a loopback catalog"), + ("https://169.254.169.254/token", "a link-local IdP"), + ("https://[::]/token", "an unspecified IdP"), + ], +) +def test_the_token_endpoint_is_judged_on_its_own(token_endpoint, refused): + raw = {**_AUTHS["oauth2"], "oauth2ServerUri": token_endpoint} + contract = _kc_contract(raw, location=_location("https://localhost:8443/catalog")) + error = iceberg_sink_preflight(contract, "ingest") + assert error is not None and "OAuth2 token endpoint" in error, refused + + +@pytest.mark.parametrize("engine_name", ["kafka-connect", "debezium"]) +def test_validate_warns_for_both_engines_and_both_types(engine_name): + build = _kc_contract if engine_name == "kafka-connect" else _dbz_contract + for auth in sorted(_AUTHS): + errors, warnings = validate_iceberg_sink(build(dict(_AUTHS[auth]))) + assert errors == [], (auth, errors) + assert any(f"{_SECRET}@{_CATALOG_HOST}" in w for w in warnings), (auth, warnings) + + +@pytest.mark.parametrize("region, refused", [("eu-west-1", False), ("eu-west-1.evil", True)]) +def test_a_glue_region_is_a_host_label(region, refused): + """Glue is where client.region builds the AWS endpoint hosts.""" + location = {"database": "bronze", "table": "orders", "bucket": "lake", "region": region} + contract = _kc_contract(None, location=location) + contract["exposes"][0]["binding"]["platform"] = "aws" + errors, _warnings = validate_iceberg_sink(contract) + assert any("binding.location.region" in e for e in errors) is refused, errors + + +def test_sink_partition_by_is_read_as_sinkspec_reads_it(): + """A mapping reaches the config as its keys (SinkSpec.from_dict lists it).""" + contract = _kc_contract(None, location=_location(partitionBy=["day"])) + contract["builds"][0]["properties"]["sink"]["partitionBy"] = {f"${{env:{_SECRET}}}": 1} + errors, _warnings = validate_iceberg_sink(contract) + assert any("sink.partitionBy" in e for e in errors), errors diff --git a/tests/integration/test_lakekeeper_oauth_live.py b/tests/integration/test_lakekeeper_oauth_live.py index 277ca0bcc..60761f869 100644 --- a/tests/integration/test_lakekeeper_oauth_live.py +++ b/tests/integration/test_lakekeeper_oauth_live.py @@ -145,6 +145,19 @@ ), ] +#: The three tests that derive a sink config. Since the Iceberg sink destination +#: policy (CHANGELOG Unreleased, Security), a ``sink.catalogAuth`` credential +#: reaches only https hosts, a loopback one included, and this stack serves +#: Lakekeeper and Keycloak over http. The founder chose (2026-11-04) to skip them +#: until the stack runs TLS (Trello XgF5xhrM, "Live Lakekeeper OAuth2 test: run +#: the stack over TLS"). The fourth test talks to Lakekeeper directly. +_NEEDS_TLS_STACK = pytest.mark.skip( + reason=( + "catalogAuth hosts must be https; this stack serves Lakekeeper and Keycloak over " + "http until the TLS follow-up lands" + ) +) + _REALM = "fluid" _CLIENT_ID = "fluid-sink" #: The client scope whose Audience mapper adds :data:`_AUDIENCE` to ``aud``. @@ -593,6 +606,7 @@ def _wait_failed(stack: _AuthStack, name: str, *, deadline_s: float = 180) -> Di # --------------------------------------------------------------------------- +@_NEEDS_TLS_STACK def test_catalog_auth_oauth2_streams_into_an_auth_required_lakekeeper( request: pytest.FixtureRequest, ): @@ -647,6 +661,7 @@ def test_catalog_auth_oauth2_streams_into_an_auth_required_lakekeeper( _delete_connector(stack, connector) +@_NEEDS_TLS_STACK def test_stored_connector_config_holds_the_reference_not_the_secret( auth_stack: _AuthStack, request: pytest.FixtureRequest ): @@ -697,6 +712,7 @@ def test_stored_connector_config_holds_the_reference_not_the_secret( _delete_connector(stack, connector) +@_NEEDS_TLS_STACK def test_unset_credential_variable_fails_the_task_and_writes_nothing( auth_stack: _AuthStack, request: pytest.FixtureRequest ): diff --git a/tests/security/test_no_dollar_anchored_validators.py b/tests/security/test_no_dollar_anchored_validators.py new file mode 100644 index 000000000..888d8c027 --- /dev/null +++ b/tests/security/test_no_dollar_anchored_validators.py @@ -0,0 +1,2190 @@ +# Copyright 2024-2026 Agentics Transformation Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Static gate: no whole-string validator in ``fluid_build/`` ends in ``$`` under ``.match``. + +Python's ``$`` matches at the end of the string and also just before a final +``\\n``. So ``re.match(r"^[a-z]+$", "orders\\n")`` succeeds, and the caller goes on +to use ``"orders\\n"``. Contract YAML produces such values easily (a ``|`` block +scalar ends in one), and so do ``.env`` files and ``%0A`` in a URL. #788 and #797 +fixed the known validators with ``\\Z`` or ``fullmatch``. This test keeps the +class from coming back. + +**The scan** walks the AST and imports nothing from ``fluid_build/``. It finds +every ``re.match`` / ``re.search`` call, and every ``.match`` / ``.search`` call on a +compiled pattern or reference to one (``filter(RX.match, names)``). It follows the +pattern back to where it was written: a literal, a module or class constant, a +dict, tuple or comprehension of patterns, an import from another module, a +helper's return value, a property. A call is reported when its pattern + +* has an alternative anchored at the start (``^`` or ``\\A``; ``.match`` anchors + there anyway) that can end at an unescaped ``$``. It is not reported under + ``MULTILINE``, or when the ``$`` follows a repeat that consumes a newline anyway + (``\\s*$``, ``[^/]*$``). The pattern is read with the parser ``re`` itself uses, + so groups, alternatives, escapes and ``VERBOSE`` read the way ``re`` reads them; or +* cannot be read: it is compiled at run time from data (a schema ``pattern``), or + it is a ``re.Pattern`` parameter, so the caller chooses it. + +A call that uses ``fullmatch``, ``sub``, ``findall`` or ``split`` is not reported, +since ``$`` cannot leave a newline unchecked there. + +**Fix a report** with ``\\Z`` or ``fullmatch``. Where the ``$`` is deliberate, add +the call to ``_ACCEPTED`` with the reason. The key is (file, function, pattern), +so a new function that reuses a listed pattern is reported again. An entry that no +longer matches a call fails ``test_every_accepted_entry_still_matches_a_call``, +so the list can only shrink. + +**Prior art.** No ruff, pylint, bandit or dlint rule covers this +(astral-sh/ruff#15017 proposes one). CodeQL's ``py/regex/unmatchable-dollar`` is +a different bug. The Semgrep registry rule ``check-validation-regex``, ported from +Brakeman's ``CheckValidationRegex``, covers Ruby only, and Semgrep CE cannot +follow a pattern across files or through a helper's return value. This test +borrows Brakeman's rule (a validator anchors at both ends of the string) and the +shape of this repo's other AST gates: ``tests/test_no_unencoded_text_io.py``, +``tests/test_no_reserved_tld_defaults.py`` and +``tests/fragments/test_no_raw_readers.py``. + +**Not covered.** An unanchored ``.search(r"\\.ya?ml$")`` is a suffix check, not a +whole-string validator, so it is not reported. The scan does not follow a pattern +into a registry filled by ``.append`` or item assignment, into a rule object +(``Rule(name, re.compile(...))``), into an attribute only a subclass sets, or +through ``+=``, ``getattr``, ``functools.partial`` or a star import, and a ``.match`` +on a parameter that is not annotated ``re.Pattern`` is not reported. +``fluid_build/`` holds no validator of these shapes today. A pattern built with +``%`` or ``str.format`` cannot be read, so it is reported as built at run time. +JSON-schema ``pattern`` keywords run inside jsonschema, out of this scan's sight: +``schema_manager.schema_validator`` reads their ``$`` as ECMA-262 does, and a +validation that does not go through it is a separate gap. +""" + +from __future__ import annotations + +import ast +import pathlib +import re +import textwrap +import warnings +from collections import Counter +from dataclasses import dataclass, field +from typing import ( + Callable, + Dict, + Iterator, + List, + NamedTuple, + Optional, + Sequence, + Set, + Tuple, + Union, +) + +import pytest + +try: # Python 3.11+: the parser ``re`` itself uses + _sre = re._constants + _sre_parse = re._parser +except AttributeError: # Python 3.10, where ``re`` is one module + import sre_constants as _sre # type: ignore[no-redef] + import sre_parse as _sre_parse # type: ignore[no-redef] + +pytestmark = [pytest.mark.unit] + +_PACKAGE = pathlib.Path(__file__).resolve().parents[2] / "fluid_build" + +#: The calls that leave a final newline unchecked when the pattern ends in ``$``. +_MATCHERS = frozenset({"match", "search"}) + +#: The modules whose ``match`` / ``search`` / ``compile`` the scan reads. +_REGEX_MODULES = frozenset({"re", "regex"}) + +#: At most this many values are kept per expression, so a long ``+`` chain stays cheap. +_MAX_VALUES = 32 + +#: At most this much of a string's start and of its end is kept. +_MAX_TEXT = 4096 + +#: Calls that hand back what they are given: ``tuple([...])``, ``MappingProxyType({...})``. +_PASS_THROUGH = frozenset( + {"dict", "frozenset", "list", "MappingProxyType", "reversed", "set", "sorted", "tuple"} +) + +_NEWLINE = ord("\n") +#: ``\s``, ``\D`` and ``\W`` contain ``\n``. +_CLASSES_WITH_NEWLINE = frozenset( + {_sre.CATEGORY_SPACE, _sre.CATEGORY_NOT_DIGIT, _sre.CATEGORY_NOT_WORD} +) +_REPEATS = frozenset( + {_sre.MAX_REPEAT, _sre.MIN_REPEAT, getattr(_sre, "POSSESSIVE_REPEAT", _sre.MAX_REPEAT)} +) +_ATOMIC_GROUP = getattr(_sre, "ATOMIC_GROUP", None) + + +# ── values ────────────────────────────────────────────────────────────── + + +@dataclass(frozen=True) +class _Str: + """A string as far as the scan can read it: a known prefix and suffix, or all of it. + + ``head`` / ``tail`` are ``None`` when nothing is known of that end.""" + + head: Optional[str] + tail: Optional[str] + whole: bool + origin: str = field(compare=False) + + @classmethod + def of(cls, text: str, origin: str) -> "_Str": + return cls(text, text, True, origin) + + def show(self) -> str: + if self.whole: + return self.head or "" + return f"{self.head or ''}…{self.tail or ''}" + + +@dataclass(frozen=True) +class _Pattern: + """A regex: the argument of ``re.compile``, or the pattern given to ``re.match``.""" + + source: Optional[_Str] # None: built at run time from a value the scan cannot read + flags: Optional[int] # None: the flags argument cannot be read + what: str = field(compare=False) # source text of the pattern argument + origin: str = field(compare=False) + + +@dataclass(frozen=True) +class _Passed: + """A parameter annotated ``re.Pattern``: the caller chooses the pattern.""" + + name: str + origin: str = field(compare=False) + + +@dataclass(frozen=True) +class _Seq: + """A tuple, list or set display, one value set per position.""" + + items: Tuple[Tuple["_Value", ...], ...] + + +@dataclass(frozen=True) +class _Map: + """A dict display. A key is ``None`` when it is not a string literal.""" + + entries: Tuple[Tuple[Optional[str], Tuple["_Value", ...]], ...] + + +@dataclass(frozen=True, eq=False) +class _ModuleRef: + name: str + + +@dataclass(frozen=True, eq=False) +class _Function: + node: Union[ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda] + scope: "_Scope" # the function's own scope + + +@dataclass(frozen=True, eq=False) +class _Class: + scope: "_Scope" # the class body + + +@dataclass(frozen=True, eq=False) +class _Instance: + scope: "_Scope" # the class body + + +_Value = Union[_Str, _Pattern, _Passed, _Seq, _Map, _ModuleRef, _Function, _Class, _Instance] + + +class _Binding(NamedTuple): + """One way a name gets its value, evaluated only when the name is read.""" + + expr: Optional[ast.expr] = None + scope: Optional["_Scope"] = None + iterate: bool = False # bound by a ``for``: the value is an element of ``expr`` + path: Tuple[int, ...] = () # the position inside a tuple target + value: Optional[_Value] = None # known when the binding is made (def, class) + imported: Optional[Tuple[str, str]] = None # (module, name) of a ``from`` import + + +class _Scope: + """The names one module, class, function or comprehension binds.""" + + def __init__(self, module: "_Module", kind: str, parent: Optional["_Scope"]) -> None: + self.module = module + self.kind = kind + self.parent = parent # the next scope a name is looked up in (classes are skipped) + self.bindings: Dict[str, List[_Binding]] = {} + self.params: Dict[str, Tuple[ast.arg, Optional[ast.expr], "_Scope"]] = {} + self.globals: Set[str] = set() + self.self_name: Optional[str] = None # a method's first parameter + self.cls: Optional["_Scope"] = None # a method's class body + self.attrs: Dict[str, List[_Binding]] = {} # a class's ``self.X = ...`` assignments + self.bases: List[Tuple[ast.expr, "_Scope"]] = [] # a class's base classes + + def bind(self, name: str, binding: _Binding) -> None: + self.bindings.setdefault(name, []).append(binding) + + +class _Module: + def __init__(self, name: str, path: pathlib.Path) -> None: + self.name = name + self.path = path + #: Held for the whole scan: the scan keys nodes by ``id()``, which a freed node gives up. + self.tree: Optional[ast.Module] = None + self.scope = _Scope(self, "module", None) + #: Every ``.match`` / ``.search`` call or reference: (node, scope, function name). + self.calls: List[Tuple[ast.expr, _Scope, str]] = [] + #: id(node) -> the scope of a comprehension or lambda. + self.scopes: Dict[int, _Scope] = {} + + +class Finding(NamedTuple): + path: str # relative to the checkout, e.g. ``fluid_build/cli/security.py`` + function: str # the qualified name of the function that makes the call + pattern: str # the pattern text, or ```` / ```` + line: int + call: str + origin: str # where the pattern was written or compiled + + @property + def key(self) -> Tuple[str, str, str]: + return (self.path, self.function, self.pattern) + + def show(self) -> str: + return ( + f"{self.path}:{self.line} in {self.function}: {self.call}\n" + f" pattern {self.pattern!r}, from {self.origin}" + ) + + +# ── reading a module ──────────────────────────────────────────────────── + + +class _Builder(ast.NodeVisitor): + """Record every binding and every ``.match`` / ``.search`` call of one module.""" + + def __init__(self, module: _Module) -> None: + self.module = module + self.scope = module.scope + self.names: List[str] = [] + self._called: Set[int] = set() # id() of the ``x.match`` nodes that are called + + # scopes + + def _lookup_parent(self) -> _Scope: + """The scope a nested function looks names up in: Python skips class bodies.""" + scope = self.scope + while scope.kind == "class": + assert scope.parent is not None + scope = scope.parent + return scope + + def _function(self, node: Union[ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda]) -> None: + outer = self.scope + for expr in getattr(node, "decorator_list", []): + self.visit(expr) + args = node.args + for expr in args.defaults + [d for d in args.kw_defaults if d is not None]: + self.visit(expr) + inner = _Scope(self.module, "function", self._lookup_parent()) + positional = args.posonlyargs + args.args + defaults: List[Optional[ast.expr]] = [None] * (len(positional) - len(args.defaults)) + defaults += list(args.defaults) + for arg, default in zip(positional, defaults, strict=True): + inner.params[arg.arg] = (arg, default, outer) + for arg, default in zip(args.kwonlyargs, args.kw_defaults, strict=True): + inner.params[arg.arg] = (arg, default, outer) + for arg in (args.vararg, args.kwarg): + if arg is not None: + inner.params[arg.arg] = (arg, None, outer) + if outer.kind == "class" and positional and not _decorated(node, "staticmethod"): + inner.self_name = positional[0].arg + inner.cls = outer + if isinstance(node, ast.Lambda): + self.module.scopes[id(node)] = inner + else: + self._bind_name(node.name, _Binding(value=_Function(node, inner))) + self.scope = inner + self.names.append(getattr(node, "name", "")) + for stmt in node.body if isinstance(node.body, list) else [node.body]: + self.visit(stmt) + self.names.pop() + self.scope = outer + + def visit_FunctionDef(self, node: ast.FunctionDef) -> None: + self._function(node) + + def visit_AsyncFunctionDef(self, node: ast.AsyncFunctionDef) -> None: + self._function(node) + + def visit_Lambda(self, node: ast.Lambda) -> None: + self._function(node) + + def visit_ClassDef(self, node: ast.ClassDef) -> None: + for expr in node.decorator_list + node.bases + [kw.value for kw in node.keywords]: + self.visit(expr) + outer = self.scope + body = _Scope(self.module, "class", self._lookup_parent()) + body.bases = [(base, outer) for base in node.bases] + self._bind_name(node.name, _Binding(value=_Class(body))) + self.scope = body + self.names.append(node.name) + for stmt in node.body: + self.visit(stmt) + self.names.pop() + self.scope = outer + + def _comprehension(self, node: ast.AST, generators: List[ast.comprehension]) -> None: + outer = self.scope + inner = _Scope(self.module, "function", self._lookup_parent()) + self.module.scopes[id(node)] = inner + for index, gen in enumerate(generators): + # The first iterable is evaluated outside the comprehension. + self.scope = outer if index == 0 else inner + self.visit(gen.iter) + self.scope = inner + self._bind_target(gen.target, gen.iter, outer if index == 0 else inner, True, ()) + for cond in gen.ifs: + self.visit(cond) + for child in ("elt", "key", "value"): + expr = getattr(node, child, None) + if expr is not None: + self.visit(expr) + self.scope = outer + + def visit_ListComp(self, node: ast.ListComp) -> None: + self._comprehension(node, node.generators) + + def visit_SetComp(self, node: ast.SetComp) -> None: + self._comprehension(node, node.generators) + + def visit_GeneratorExp(self, node: ast.GeneratorExp) -> None: + self._comprehension(node, node.generators) + + def visit_DictComp(self, node: ast.DictComp) -> None: + self._comprehension(node, node.generators) + + # bindings + + def _bind_name(self, name: str, binding: _Binding) -> None: + if name in self.scope.globals: + self.module.scope.bind(name, binding) + else: + self.scope.bind(name, binding) + + def _bind_target( + self, + target: ast.expr, + expr: ast.expr, + scope: _Scope, + iterate: bool, + path: Tuple[int, ...], + ) -> None: + if isinstance(target, ast.Name): + self._bind_name(target.id, _Binding(expr, scope, iterate, path)) + elif isinstance(target, (ast.Tuple, ast.List)): + for index, elt in enumerate(target.elts): + if not isinstance(elt, ast.Starred): + self._bind_target(elt, expr, scope, iterate, path + (index,)) + elif ( + isinstance(target, ast.Attribute) + and isinstance(target.value, ast.Name) + and target.value.id == self.scope.self_name + and self.scope.cls is not None + ): + binding = _Binding(expr, scope, iterate, path) + self.scope.cls.attrs.setdefault(target.attr, []).append(binding) + + def visit_Assign(self, node: ast.Assign) -> None: + self.visit(node.value) + for target in node.targets: + self._bind_target(target, node.value, self.scope, False, ()) + self.visit(target) + + def visit_AnnAssign(self, node: ast.AnnAssign) -> None: + if node.value is not None: + self.visit(node.value) + self._bind_target(node.target, node.value, self.scope, False, ()) + + def visit_NamedExpr(self, node: ast.NamedExpr) -> None: + self.visit(node.value) + self._bind_target(node.target, node.value, self.scope, False, ()) + + def visit_For(self, node: Union[ast.For, ast.AsyncFor]) -> None: + self.visit(node.iter) + self._bind_target(node.target, node.iter, self.scope, True, ()) + for stmt in node.body + node.orelse: + self.visit(stmt) + + visit_AsyncFor = visit_For + + def visit_Global(self, node: ast.Global) -> None: + self.scope.globals.update(node.names) + + def visit_Import(self, node: ast.Import) -> None: + for alias in node.names: + if alias.asname: + self._bind_name(alias.asname, _Binding(value=_ModuleRef(alias.name))) + else: + top = alias.name.split(".")[0] + self._bind_name(top, _Binding(value=_ModuleRef(top))) + + def visit_ImportFrom(self, node: ast.ImportFrom) -> None: + base = node.module or "" + if node.level: + package = self.module.name.split(".") + if self.module.path.name != "__init__.py": + package = package[:-1] + package = package[: len(package) - (node.level - 1)] + base = ".".join(package + ([base] if base else [])) + for alias in node.names: + if alias.name != "*": + binding = _Binding(imported=(base, alias.name)) + self._bind_name(alias.asname or alias.name, binding) + + # calls + + def _record(self, node: ast.expr) -> None: + self.module.calls.append((node, self.scope, ".".join(self.names) or "")) + + def visit_Call(self, node: ast.Call) -> None: + if isinstance(node.func, ast.Attribute) and node.func.attr in _MATCHERS: + self._record(node) + self._called.add(id(node.func)) + self.generic_visit(node) + + def visit_Attribute(self, node: ast.Attribute) -> None: + loaded = isinstance(node.ctx, ast.Load) + if node.attr in _MATCHERS and loaded and id(node) not in self._called: + self._record(node) # a reference, such as ``filter(RX.match, names)`` + self.generic_visit(node) + + +# ── following a value back ────────────────────────────────────────────── + + +def _dedupe(values: Sequence[_Value]) -> List[_Value]: + out: List[_Value] = [] + for value in values: + if value not in out: + out.append(value) + if len(out) >= _MAX_VALUES: + break + return out + + +def _concat(left: _Str, right: _Str, origin: str) -> _Str: + """``left + right``, keeping what is known of the start and of the end.""" + if left.whole and right.whole: + text = f"{left.head}{right.head}" + if len(text) <= _MAX_TEXT: + return _Str.of(text, origin) + return _Str(text[:_MAX_TEXT], text[-_MAX_TEXT:], False, origin) + head, tail = left.head, right.tail + if left.whole: + head = f"{left.head}{right.head}" if right.head is not None else left.head or None + if right.whole: + tail = f"{left.tail}{right.tail}" if left.tail is not None else right.tail or None + return _Str( + head[:_MAX_TEXT] if head is not None else None, + tail[-_MAX_TEXT:] if tail is not None else None, + False, + origin, + ) + + +def _is_regex_module(values: Sequence[_Value]) -> bool: + return any(isinstance(v, _ModuleRef) and v.name in _REGEX_MODULES for v in values) + + +def _decorated(node: ast.AST, *names: str) -> bool: + """Whether ``node`` carries a decorator called one of ``names`` (``functools.x`` too).""" + for decorator in getattr(node, "decorator_list", []): + if isinstance(decorator, ast.Call): + decorator = decorator.func + if getattr(decorator, "id", None) in names or getattr(decorator, "attr", None) in names: + return True + return False + + +def _keyword(call: ast.Call, name: str) -> Optional[ast.expr]: + return next((kw.value for kw in call.keywords if kw.arg == name), None) + + +def _own_returns(node: ast.AST) -> Iterator[ast.Return]: + """The ``return`` statements of a function, not of the functions nested in it.""" + stack = list(ast.iter_child_nodes(node)) + while stack: + child = stack.pop() + if isinstance(child, ast.Return) and child.value is not None: + yield child + elif not isinstance( + child, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda, ast.ClassDef) + ): + stack.extend(ast.iter_child_nodes(child)) + + +class _Scan: + def __init__(self, package: pathlib.Path) -> None: + self.package = package + self.checkout = package.parent + self._modules: Dict[str, Optional[_Module]] = {} + self._active: Set[Tuple[int, str]] = set() + self._memo: Dict[Tuple[int, str], List[_Value]] = {} + self._cut = False # a cycle guard cut the evaluation in progress short + self.calls_seen = 0 + self.unreadable: List[str] = [] + + # modules + + def _rel(self, path: pathlib.Path) -> str: + return path.relative_to(self.checkout).as_posix() + + def _origin(self, scope: _Scope, node: ast.AST) -> str: + return f"{self._rel(scope.module.path)}:{getattr(node, 'lineno', 0)}" + + def _path_of(self, name: str) -> Optional[pathlib.Path]: + if name != self.package.name and not name.startswith(self.package.name + "."): + return None + rel = pathlib.Path(*name.split(".")) + for candidate in ( + self.checkout / rel.with_suffix(".py"), + self.checkout / rel / "__init__.py", + ): + if candidate.is_file(): + return candidate + return None + + def module(self, name: str) -> Optional[_Module]: + if name not in self._modules: + path = self._path_of(name) + self._modules[name] = self._load(name, path) if path else None + return self._modules[name] + + def _load(self, name: str, path: pathlib.Path) -> Optional[_Module]: + module = _Module(name, path) + try: + with warnings.catch_warnings(): + # An invalid escape sequence is the linter's business, not this gate's. + warnings.simplefilter("ignore") + module.tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + _Builder(module).visit(module.tree) + except (SyntaxError, UnicodeDecodeError, ValueError, RecursionError) as exc: + self.unreadable.append(f"{self._rel(path)}: {type(exc).__name__}: {exc}") + return None + return module + + # names + + def _guarded(self, key: Tuple[int, str], compute: Callable[[], List[_Value]]) -> List[_Value]: + """``compute()`` once per key. A key met again while it is being computed reads as + nothing, and a result cut short that way is not kept. Callers get a copy.""" + if key in self._memo: + return list(self._memo[key]) + if key in self._active: + self._cut = True + return [] + outer_cut, self._cut = self._cut, False + self._active.add(key) + try: + result = compute() + if not self._cut: + self._memo[key] = result + return list(result) + finally: + self._active.discard(key) + self._cut = outer_cut or self._cut + + def _name(self, name: str, scope: _Scope) -> List[_Value]: + current: Optional[_Scope] = scope + while current is not None: + if name in current.params or name in current.bindings: + return self._bound(current, name) + current = current.parent + return [] + + def _bound(self, scope: _Scope, name: str) -> List[_Value]: + return self._guarded((id(scope), name), lambda: self._bound_now(scope, name)) + + def _bound_now(self, scope: _Scope, name: str) -> List[_Value]: + out: List[_Value] = [] + if name in scope.params: + arg, default, outer = scope.params[name] + if name == scope.self_name and scope.cls is not None: + return [_Instance(scope.cls)] + if arg.annotation is not None and self._names_pattern(arg.annotation, outer): + out.append(_Passed(name, self._origin(scope, arg))) + out += self.values(default, outer) + for binding in scope.bindings.get(name, []): + out += self._evaluate(binding) + return _dedupe(out) + + def _names_pattern(self, annotation: ast.expr, scope: _Scope) -> bool: + """Whether an annotation names ``re.Pattern`` / ``typing.Pattern``, or an alias of it.""" + if isinstance(annotation, ast.Constant) and isinstance(annotation.value, str): + try: + annotation = ast.parse(annotation.value, mode="eval").body + except SyntaxError: + return False + for node in ast.walk(annotation): + if isinstance(node, ast.Attribute) and node.attr == "Pattern": + return True + if isinstance(node, ast.Name) and ( + node.id == "Pattern" or self._imports_pattern(node.id, scope) + ): + return True + return False + + @staticmethod + def _imports_pattern(name: str, scope: _Scope) -> bool: + current: Optional[_Scope] = scope + while current is not None and name not in current.bindings: + current = current.parent + bindings = current.bindings[name] if current is not None else [] + return any(b.imported is not None and b.imported[1] == "Pattern" for b in bindings) + + def _evaluate(self, binding: _Binding) -> List[_Value]: + if binding.value is not None: + return [binding.value] + if binding.imported is not None: + return self._module_attr(*binding.imported) + values = self.values(binding.expr, binding.scope) + if binding.iterate: + values = self._elements(values) + for index in binding.path: + values = [ + item + for value in values + if isinstance(value, _Seq) and index < len(value.items) + for item in value.items[index] + ] + return values + + def _module_attr(self, module_name: str, attr: str) -> List[_Value]: + """``module.attr``: what the module binds, or else its submodule of that name.""" + module = self.module(module_name) + out: List[_Value] = [] + if module is not None and attr in module.scope.bindings: + out = self._bound(module.scope, attr) + if not out and self._path_of(f"{module_name}.{attr}") is not None: + # Also the case for ``from . import sub`` in a package's own ``__init__``. + out = [_ModuleRef(f"{module_name}.{attr}")] + return out + + def _class_attr(self, body: _Scope, attr: str) -> List[_Value]: + return self._guarded((id(body), f".{attr}"), lambda: self._class_attr_now(body, attr)) + + def _class_attr_now(self, body: _Scope, attr: str) -> List[_Value]: + out: List[_Value] = [] + if attr in body.bindings: + out += self._bound(body, attr) + for binding in body.attrs.get(attr, []): + out += self._evaluate(binding) + for base, scope in body.bases: + if isinstance(base, ast.Subscript): # a generic base, ``Base[int]`` + base = base.value + for value in self.values(base, scope): + if isinstance(value, _Class): + out += self._class_attr(value.scope, attr) + return _dedupe(out) + + def _attribute(self, bases: Sequence[_Value], attr: str) -> List[_Value]: + out: List[_Value] = [] + for base in bases: + if isinstance(base, _ModuleRef): + out += self._module_attr(base.name, attr) + elif isinstance(base, _Pattern) and attr == "pattern" and base.source is not None: + out.append(base.source) + elif isinstance(base, _Instance) and attr == "__class__": + out.append(_Class(base.scope)) + elif isinstance(base, (_Class, _Instance)): + for value in self._class_attr(base.scope, attr): + is_property = isinstance(value, _Function) and _decorated( + value.node, "property", "cached_property" + ) + if is_property and isinstance(base, _Instance): + out += self._returns(value) + else: + out.append(value) + return _dedupe(out) + + @staticmethod + def _elements(values: Sequence[_Value]) -> List[_Value]: + return _dedupe( + [item for v in values if isinstance(v, _Seq) for pos in v.items for item in pos] + ) + + # expressions: one reader per node type, dispatched the way ast.NodeVisitor does + + def values(self, expr: Optional[ast.expr], scope: Optional[_Scope]) -> List[_Value]: + """Every value ``expr`` can have that the scan can read, flow-insensitively.""" + if expr is None or scope is None: + return [] + reader = getattr(self, f"_read_{type(expr).__name__}", None) + return reader(expr, scope) if reader is not None else [] + + def _read_Constant(self, expr: ast.Constant, scope: _Scope) -> List[_Value]: + value = expr.value + if isinstance(value, bytes): + value = value.decode("latin-1") + return [_Str.of(value, self._origin(scope, expr))] if isinstance(value, str) else [] + + def _read_JoinedStr(self, expr: ast.JoinedStr, scope: _Scope) -> List[_Value]: + origin = self._origin(scope, expr) + result: Optional[_Str] = None + for part in expr.values: + if isinstance(part, ast.Constant) and isinstance(part.value, str): + piece = _Str.of(part.value, origin) + else: + inner: List[_Str] = [] + if ( + isinstance(part, ast.FormattedValue) + and part.format_spec is None + and part.conversion == -1 + ): + inner = [v for v in self.values(part.value, scope) if isinstance(v, _Str)] + piece = inner[0] if len(inner) == 1 else _Str(None, None, False, origin) + result = piece if result is None else _concat(result, piece, origin) + return [result if result is not None else _Str.of("", origin)] + + def _read_BinOp(self, expr: ast.BinOp, scope: _Scope) -> List[_Value]: + if not isinstance(expr.op, ast.Add): + return [] + origin = self._origin(scope, expr) + lefts = [v for v in self.values(expr.left, scope) if isinstance(v, _Str)] + rights = [v for v in self.values(expr.right, scope) if isinstance(v, _Str)] + return _dedupe([_concat(a, b, origin) for a in lefts for b in rights]) + + def _read_Name(self, expr: ast.Name, scope: _Scope) -> List[_Value]: + return self._name(expr.id, scope) + + def _read_Attribute(self, expr: ast.Attribute, scope: _Scope) -> List[_Value]: + return self._attribute(self.values(expr.value, scope), expr.attr) + + def _read_Subscript(self, expr: ast.Subscript, scope: _Scope) -> List[_Value]: + index = expr.slice + key = index.value if isinstance(index, ast.Constant) else None + out: List[_Value] = [] + for base in self.values(expr.value, scope): + if isinstance(base, _Map): + out += self._lookup(base, key if isinstance(key, str) else None) + elif isinstance(base, _Seq): + known = isinstance(key, int) and -len(base.items) <= key < len(base.items) + if known and len(base.items) > 1: + out += base.items[key] + else: + out += [v for pos in base.items for v in pos] + return _dedupe(out) + + @staticmethod + def _lookup(mapping: _Map, key: Optional[str]) -> List[_Value]: + hits = [vs for k, vs in mapping.entries if key is not None and k == key] + chosen = hits if hits else [vs for _, vs in mapping.entries] + return _dedupe([v for vs in chosen for v in vs]) + + def _read_Dict(self, expr: ast.Dict, scope: _Scope) -> List[_Value]: + entries: List[Tuple[Optional[str], Tuple[_Value, ...]]] = [] + for key, value in zip(expr.keys, expr.values, strict=True): + if key is None: # ``**other`` + entries += [ + e for m in self.values(value, scope) if isinstance(m, _Map) for e in m.entries + ] + continue + text = key.value if isinstance(key, ast.Constant) else None + entries.append( + (text if isinstance(text, str) else None, tuple(self.values(value, scope))) + ) + return [_Map(tuple(entries))] + + def _read_Tuple(self, expr: Union[ast.Tuple, ast.List, ast.Set], scope: _Scope) -> List[_Value]: + items: List[Tuple[_Value, ...]] = [] + for elt in expr.elts: + if isinstance(elt, ast.Starred): + items.append(tuple(self._elements(self.values(elt.value, scope)))) + else: + items.append(tuple(self.values(elt, scope))) + keeps_positions = isinstance(expr, ast.Tuple) and not any( + isinstance(elt, ast.Starred) for elt in expr.elts + ) + if not keeps_positions: + items = [tuple(_dedupe([v for pos in items for v in pos]))] + return [_Seq(tuple(items))] + + _read_List = _read_Set = _read_Tuple + + def _read_ListComp( + self, expr: Union[ast.ListComp, ast.SetComp, ast.GeneratorExp], scope: _Scope + ) -> List[_Value]: + inner = scope.module.scopes.get(id(expr)) + return [_Seq((tuple(self.values(expr.elt, inner)),))] if inner else [] + + _read_SetComp = _read_GeneratorExp = _read_ListComp + + def _read_DictComp(self, expr: ast.DictComp, scope: _Scope) -> List[_Value]: + inner = scope.module.scopes.get(id(expr)) + return [_Map(((None, tuple(self.values(expr.value, inner))),))] if inner else [] + + def _read_Lambda(self, expr: ast.Lambda, scope: _Scope) -> List[_Value]: + inner = scope.module.scopes.get(id(expr)) + return [_Function(expr, inner)] if inner else [] + + def _read_IfExp(self, expr: ast.IfExp, scope: _Scope) -> List[_Value]: + return _dedupe(self.values(expr.body, scope) + self.values(expr.orelse, scope)) + + def _read_BoolOp(self, expr: ast.BoolOp, scope: _Scope) -> List[_Value]: + return _dedupe([v for operand in expr.values for v in self.values(operand, scope)]) + + def _read_NamedExpr(self, expr: ast.NamedExpr, scope: _Scope) -> List[_Value]: + return self.values(expr.value, scope) + + # calls + + def _read_Call(self, expr: ast.Call, scope: _Scope) -> List[_Value]: + func = expr.func + if isinstance(func, ast.Attribute): + bases = self.values(func.value, scope) + if _is_regex_module(bases): + return self._regex_call(func.attr, expr, scope) + known = self._method(func.attr, bases, expr, scope) + if known is not None: + return known + # Read the callee from ``bases``: reading ``func`` again would evaluate the + # receiver twice, which doubles the work at every call in a chain. + callees, name = self._attribute(bases, func.attr), func.attr + else: + callees, name = self.values(func, scope), getattr(func, "id", "") + if not callees: + return self._builtin(name, expr, scope) + out: List[_Value] = [] + for callee in callees: + if isinstance(callee, _Function): + out += self._returns(callee) + elif isinstance(callee, _Class): + out.append(_Instance(callee.scope)) + return _dedupe(out) + + def _regex_call(self, attr: str, expr: ast.Call, scope: _Scope) -> List[_Value]: + if attr == "compile": + return self._compiled(expr, scope, flags_at=1) + if attr == "escape": + # An escaped string anchors nothing: its ``$`` comes out as ``\$``. + return [_Str("", "", False, self._origin(scope, expr))] + return [] + + def _method( + self, attr: str, bases: Sequence[_Value], expr: ast.Call, scope: _Scope + ) -> Optional[List[_Value]]: + """``str.join``, and the dict methods that hand back what the dict holds.""" + separators = [b for b in bases if isinstance(b, _Str)] + if attr == "join" and separators and expr.args: + parts = self._elements(self.values(expr.args[0], scope)) + return self._joined(separators, parts, self._origin(scope, expr)) + maps = [b for b in bases if isinstance(b, _Map)] + if not maps: + return None + if attr == "get" and expr.args: + key = expr.args[0] + text = key.value if isinstance(key, ast.Constant) else None + out = [ + v for m in maps for v in self._lookup(m, text if isinstance(text, str) else None) + ] + return _dedupe(out + self.values(expr.args[1] if len(expr.args) > 1 else None, scope)) + if attr == "values": + return [_Seq((tuple(_dedupe([v for m in maps for _, vs in m.entries for v in vs])),))] + if attr == "items": + origin = self._origin(scope, expr) + pairs = [ + _Seq(((_Str.of(k, origin),) if k else (), vs)) for m in maps for k, vs in m.entries + ] + return [_Seq((tuple(pairs),))] + return None + + @staticmethod + def _joined(separators: List[_Str], parts: List[_Value], origin: str) -> List[_Value]: + """``sep.join(parts)``. Joined by ``|``, the parts are the alternatives of one + pattern, so each is read on its own. Otherwise the result starts and ends the + way the parts do, where they agree.""" + strings = [p for p in parts if isinstance(p, _Str)] + if not strings or len(strings) != len(parts): + return [] + if all(s.whole and s.head == "|" for s in separators): + return list(strings) + heads = {s.head for s in strings} + tails = {s.tail for s in strings} + head = heads.pop() if len(heads) == 1 else None + tail = tails.pop() if len(tails) == 1 else None + return [_Str(head, tail, False, origin)] + + def _builtin(self, name: str, expr: ast.Call, scope: _Scope) -> List[_Value]: + """The builtins (and ``MappingProxyType``) that pass a container through.""" + first = expr.args[0] if expr.args else None + if name == "type" and first is not None: + return [_Class(v.scope) for v in self.values(first, scope) if isinstance(v, _Instance)] + if name == "enumerate" and first is not None: + elements = self._elements(self.values(first, scope)) + return [_Seq((tuple(_Seq(((), (e,))) for e in elements),))] + if name not in _PASS_THROUGH: + return [] + out = self.values(first, scope) + if name == "dict" and expr.keywords: + keywords = [(kw.arg, tuple(self.values(kw.value, scope))) for kw in expr.keywords] + out.append(_Map(tuple(keywords))) + return out + + def _compiled(self, expr: ast.Call, scope: _Scope, flags_at: int) -> List[_Value]: + """The pattern(s) ``re.compile`` / ``re.match`` / ``re.search`` is given.""" + pattern = expr.args[0] if expr.args else _keyword(expr, "pattern") + flags_expr = expr.args[flags_at] if len(expr.args) > flags_at else _keyword(expr, "flags") + flags = 0 if flags_expr is None else self._flags(flags_expr, scope) + what = ast.unparse(pattern) if pattern is not None else "?" + origin = self._origin(scope, expr) + out: List[_Value] = [] + for value in self.values(pattern, scope): + if isinstance(value, (_Pattern, _Passed)): + out.append(value) + elif isinstance(value, _Str) and value.tail is not None: + out.append(_Pattern(value, flags, what, value.origin)) + elif isinstance(value, _Str): + # A string whose end the scan cannot read counts as built at run time. + out.append(_Pattern(None, flags, what, origin)) + if not out: + out.append(_Pattern(None, flags, what, origin)) + return _dedupe(out) + + def _flags(self, expr: ast.expr, scope: _Scope) -> Optional[int]: + if isinstance(expr, ast.Constant) and isinstance(expr.value, int): + return expr.value + if isinstance(expr, ast.BinOp) and isinstance(expr.op, ast.BitOr): + left, right = self._flags(expr.left, scope), self._flags(expr.right, scope) + return None if left is None or right is None else left | right + if isinstance(expr, ast.Attribute) and _is_regex_module(self.values(expr.value, scope)): + flag = getattr(re, expr.attr, None) + return int(flag) if isinstance(flag, int) else None + if isinstance(expr, ast.Name): + current: Optional[_Scope] = scope + while current is not None and expr.id not in current.bindings: + current = current.parent + key = (id(current), f"") + if current is None or key in self._active: + return None + self._active.add(key) + try: + found = { + self._flags(b.expr, b.scope) + for b in current.bindings[expr.id] + if b.expr is not None and b.scope is not None and not b.path + } + finally: + self._active.discard(key) + if len(found) == 1: + return found.pop() + return None + + def _returns(self, function: _Function) -> List[_Value]: + key = (id(function.node), "") + return self._guarded(key, lambda: self._returns_now(function)) + + def _returns_now(self, function: _Function) -> List[_Value]: + node = function.node + if isinstance(node, ast.Lambda): + return self.values(node.body, function.scope) + out: List[_Value] = [] + for ret in _own_returns(node): + out += self.values(ret.value, function.scope) + return _dedupe(out) + + # the check + + def findings(self) -> List[Finding]: + out: List[Finding] = [] + for path in sorted(self.package.rglob("*.py")): + if "__pycache__" in path.parts: + continue + try: + text = path.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError) as exc: + self.unreadable.append(f"{self._rel(path)}: {type(exc).__name__}: {exc}") + continue + if ".match" not in text and ".search" not in text: + continue + rel = path.relative_to(self.checkout).with_suffix("") + parts = list(rel.parts[:-1]) if rel.name == "__init__" else list(rel.parts) + module = self.module(".".join(parts)) + if module is None: + continue + for node, scope, function in module.calls: + out += self._check(module, node, scope, function) + return sorted(set(out)) + + def _check( + self, module: _Module, node: ast.expr, scope: _Scope, function: str + ) -> List[Finding]: + func = node.func if isinstance(node, ast.Call) else node + assert isinstance(func, ast.Attribute) + rel = self._rel(module.path) + try: + bases = self.values(func.value, scope) + if not _is_regex_module(bases): + patterns = [b for b in bases if isinstance(b, (_Pattern, _Passed))] + elif isinstance(node, ast.Call): + patterns = self._compiled(node, scope, flags_at=2) + else: + patterns = [] # ``re.match`` handed on: its pattern is given elsewhere + except RecursionError: + self.unreadable.append(f"{rel}:{node.lineno}: too deeply nested to follow") + return [] + if patterns: + self.calls_seen += 1 + shown = ast.unparse(node) + if len(shown) > 100: + shown = shown[:97] + "..." + out = [] + for pattern in patterns: + label = _gap(pattern, func.attr) + if label is not None: + out.append(Finding(rel, function, label, node.lineno, shown, pattern.origin)) + return out + + +# ── the anchor rule ───────────────────────────────────────────────────── +# +# Items come from ``re``'s own parser: ``(opcode, argument)`` pairs, where a group, +# a repeat or an alternation holds parsed sub-patterns of its own. + + +def _matches_newline(item: Tuple[object, object], flags: int) -> bool: + """Whether one parsed character item (``.``, ``\\n``, ``[^/]``, ``\\s``) matches ``\\n``.""" + op, av = item + if op is _sre.ANY: + return bool(flags & re.DOTALL) + if op is _sre.LITERAL: + return av == _NEWLINE + if op is _sre.NOT_LITERAL: + return av != _NEWLINE + if op is _sre.IN: + negated = any(sub_op is _sre.NEGATE for sub_op, _ in av) + hit = any( + (sub_op is _sre.LITERAL and sub_av == _NEWLINE) + or (sub_op is _sre.RANGE and sub_av[0] <= _NEWLINE <= sub_av[1]) + or (sub_op is _sre.CATEGORY and sub_av in _CLASSES_WITH_NEWLINE) + for sub_op, sub_av in av + ) + return hit != negated + return False + + +def _absorbs_newline(items: List[Tuple[object, object]], flags: int) -> bool: + """True when the last item repeats, without bound, something that matches ``\\n``.""" + if not items: + return False + op, av = items[-1] + if op in _REPEATS: + _, high, sub = av + return ( + high == _sre.MAXREPEAT and len(sub.data) == 1 and _matches_newline(sub.data[0], flags) + ) + if op is _sre.SUBPATTERN: + _, add, delete, sub = av + return _absorbs_newline(sub.data, (flags | add) & ~delete) + if op is _sre.BRANCH: + return all(_absorbs_newline(branch.data, flags) for branch in av[1]) + return False + + +def _ends_at_dollar( + items: List[Tuple[object, object]], flags: int, before: Sequence[Tuple[object, object]] = () +) -> bool: + """True when a match can end at a ``$`` that lets a final newline through. + + ``before`` holds the items ahead of ``items`` in the enclosing sequence: what + precedes a ``$`` that opens a group or an alternative.""" + if not items: + return False + op, av = items[-1] + preceding = [*before, *items[:-1]] + if op is _sre.AT and av is _sre.AT_END: + return not flags & re.MULTILINE and not _absorbs_newline(preceding, flags) + if op is _sre.SUBPATTERN: + _, add, delete, sub = av + return _ends_at_dollar(sub.data, (flags | add) & ~delete, preceding) + if op is _sre.BRANCH: + return any(_ends_at_dollar(branch.data, flags, preceding) for branch in av[1]) + if op is _ATOMIC_GROUP: + return _ends_at_dollar(av.data, flags, preceding) + return False + + +def _anchored_at_start(items: List[Tuple[object, object]]) -> bool: + if not items: + return False + op, av = items[0] + if op is _sre.AT: + return av in (_sre.AT_BEGINNING, _sre.AT_BEGINNING_STRING) + if op is _sre.SUBPATTERN: + return _anchored_at_start(av[3].data) + if op is _sre.BRANCH: + return all(_anchored_at_start(branch.data) for branch in av[1]) + return False + + +def _alternatives( + items: List[Tuple[object, object]], flags: int +) -> Iterator[Tuple[List[Tuple[object, object]], int]]: + """The top-level alternatives, looking into a group that wraps the whole pattern.""" + if len(items) == 1: + op, av = items[0] + if op is _sre.BRANCH: + for branch in av[1]: + yield from _alternatives(branch.data, flags) + return + if op is _sre.SUBPATTERN: + _, add, delete, sub = av + yield from _alternatives(sub.data, (flags | add) & ~delete) + return + yield items, flags + + +def _gap(pattern: Union[_Pattern, _Passed], method: str) -> Optional[str]: + """The finding label when this pattern lets a final newline through ``method``.""" + if isinstance(pattern, _Passed): + return f"" + source = pattern.source + if source is None: + return f"" + # A string read only in part stands an inert group in for the part it cannot read. + text = source.head if source.whole else f"{source.head or ''}(?:x){source.tail or ''}" + try: + parsed = _sre_parse.parse(text or "", pattern.flags or 0) + except (re.error, OverflowError, RecursionError): + return source.show() if (source.tail or "").endswith("$") else None + for items, flags in _alternatives(parsed.data, parsed.state.flags): + anchored = method == "match" or _anchored_at_start(items) + if anchored and _ends_at_dollar(items, flags): + return source.show() + return None + + +# ── the gate ──────────────────────────────────────────────────────────── + +#: Calls whose ``$`` stays, keyed by (file, function, pattern), each with its reason. +_ACCEPTED: Dict[Tuple[str, str, str], str] = { + # The value cannot end in a newline when it is matched: it was stripped, split + # or rewritten just before, and that value is the one used afterwards. + ( + "fluid_build/__init__.py", + "_version_from_git_checkout", + r"(?P[^-]+)-(?P\d+)-g(?P[0-9a-f]+)$", + ): "stripped `git describe` output; only the captured tag and distance are used", + ( + "fluid_build/build_runners/_databricks_load.py", + "staging_volume", + r"^/Volumes/([^/]+)/([^/]+)/([^/]+)/?$", + ): "matches the stripped variable, and each captured part must then pass validate_ident", + ( + "fluid_build/build_runners/_late_arrival.py", + "parse_iso_duration", + r"^P(?:(?P\d+)D)?(?:T(?:(?P\d+)H)?(?:(?P\d+)M)?" + r"(?:(?P\d+(?:\.\d+)?)S)?)?$", + ): "matches text.strip(); the span is built from the captured numbers", + ( + "fluid_build/cli/_acquisition_stage_ext.py", + "_validate_cron", + r"^[0-9*/,\-?LW#]+$", + ): "each field comes from value.split()", + ( + "fluid_build/cli/forge_db_tools.py", + "_resolve_connection_uri", + r"^[A-Za-z0-9_]+$", + ): "matches the stripped connection name, which is the one used", + ( + "fluid_build/cli/import_workflow/dbt.py", + "_safe_tags", + r"^[a-z0-9][a-z0-9-]*[a-z0-9]$|^[a-z0-9]$", + ): "every run of characters outside [a-z0-9] was replaced with '-' just before", + ( + "fluid_build/cli/provider_init.py", + "run", + r"^[a-z][a-z0-9_]*$", + ): "matches the stripped provider name, which is the one used", + ( + "fluid_build/cli/schedule_sync.py", + "_parse_commit_author", + r"^(.+?)\s*<([^<>@\s]+@[^<>@\s]+)>$", + ): "the author is stripped, and control characters (\\n too) are refused just before", + ( + "fluid_build/copilot/agents/ai_ready_agent.py", + "_normalize_allowed_models", + r"^[a-z0-9][a-z0-9-_.]*[a-z0-9]$|^[a-z0-9]$", + ): "the token was stripped and its whitespace replaced with '-' just before", + ( + "fluid_build/copilot/missions/spec.py", + "load_mission_spec_from_path", + r"^[a-z0-9][a-z0-9_-]*$", + ): "matches the stripped mission name, which is the one kept", + ( + "fluid_build/engines/dbt/_types.py", + "split_parameterized", + r"^([A-Za-z_][A-Za-z0-9_ ]*?)\s*\(\s*([^()]*)\s*\)$", + ): "matches fluid_type.strip() and returns the captured parts", + ( + "fluid_build/exporters/sodacl.py", + "_convert_dq_rule", + r"^[A-Za-z_][A-Za-z0-9_]*$", + ): "the selector is stripped where it is read", + ( + "fluid_build/forge/federation.py", + "_secret_ref_allowlist", + r"^(?:.*@)?(?:\[[^\]]+\]|[^:\[\]]+):\d+$", + ): "matches the stripped host, which is the one kept", + ( + "fluid_build/iac/provider_match.py", + "detect_cloud_declarations", + r"^[a-z]{2}-[a-z]+-\d+$", + ): "matches region.strip(), and a match only records that AWS is declared", + ( + "fluid_build/iac/providers/databricks_jobs.py", + "_notifications", + r"^[^@\s]+@[^@\s]+\.[^@\s]+$", + ): "the target is stripped where it is read", + ( + "fluid_build/iac/providers/databricks_jobs.py", + "_run_as", + r"^[0-9a-fA-F]{8}-([0-9a-fA-F]{4}-){3}[0-9a-fA-F]{12}$", + ): "the account is stripped where it is read", + ( + "fluid_build/iac/providers/databricks_jobs.py", + "_seconds", + r"^(\d+)\s*([smhd]?)$", + ): "the text is stripped; the seconds come from the captured numbers", + ( + "fluid_build/iac/providers/databricks_jobs.py", + "_seconds", + r"^PT(?:(\d+)H)?(?:(\d+)M)?(?:(\d+)S)?$", + ): "the text is stripped; the seconds come from the captured numbers", + ( + "fluid_build/iac/providers/databricks_retention.py", + "iso_duration_seconds", + r"^P(?!$)(?:(\d{1,12})Y)?(?:(\d{1,12})M)?(?:(\d{1,12})W)?(?:(\d{1,12})D)?" + r"(?:T(?=\d)(?:(\d{1,12})H)?(?:(\d{1,12})M)?(?:(\d{1,12})S)?)?$", + ): "matches text.strip(); the seconds come from the captured numbers", + ( + "fluid_build/llm/providers.py", + "_parse_param_size", + r"^([\d.]+)\s*([BM]?)$", + ): "the text is stripped; the size comes from the captured number", + ( + "fluid_build/providers/__init__.py", + "_is_valid_name", + r"^[a-z0-9_]+$", + ): "both callers pass _normalize_name() output, which is stripped", + ( + "fluid_build/providers/_path_safety.py", + "safe_filename_stem", + r"^[A-Za-z0-9_][A-Za-z0-9_.-]*[A-Za-z0-9_]$|^[A-Za-z0-9_]$", + ): "matches the stripped text and returns that", + ( + "fluid_build/providers/gcp/util/names.py", + "normalize_bucket_name", + r"^[a-z0-9][a-z0-9\-]*[a-z0-9]$", + ): "every character outside [a-z0-9-] was replaced with '-' just before", + ( + "fluid_build/providers/odcs/mappers/sla.py", + "_as_availability_pct", + r"^(100(\.0+)?|\d{2}(\.\d+)?|\d{1}\d(\.\d+)?)%$", + ): "matches stripped text, or a percentage formatted from a float", + ( + "fluid_build/providers/odcs/mappers/sla.py", + "_as_iso_duration", + r"^P(?!$)(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?$", + ): "matches value.strip() and returns that", + ( + "fluid_build/providers/quality_engine.py", + "_parse_duration_seconds", + r"^(\d+)\s*([smhd])$", + ): "the text is stripped; the seconds come from the captured number", + ( + "fluid_build/providers/quality_engine.py", + "_parse_duration_seconds", + r"^P(\d+)W$", + ): "the text is stripped; the seconds come from the captured number", + ( + "fluid_build/providers/snowflake/plan/planner.py", + "_validate_policy_signature", + r"^\(\s*[A-Za-z_][A-Za-z0-9_]*\s+[A-Za-z_][A-Za-z0-9_(),\s]*\)" + r"(\s+RETURNS\s+[A-Za-z_][A-Za-z0-9_]*)?$", + ): "matches the stripped signature and returns that", + ( + "fluid_build/schema_manager.py", + "SchemaVersion.parse", + r"^(\d+)\.(\d+)(?:\.(\d+))?(?:-(.+))?$", + ): "matches version_str.strip() and keeps the captured parts", + ( + "fluid_build/util/cron.py", + "_values", + r"^(\*|[A-Za-z0-9]+(?:-[A-Za-z0-9]+)?)(?:/([0-9]+))?$", + ): "the expression's whitespace is collapsed and it is split into fields first", + # The value is another function's output, which cannot end in a newline. + ( + "fluid_build/iac/providers/aws.py", + "_hive_decimal", + r"^\((\d+)(?:,(\d+))?\)$", + ): "the only caller passes column_types.split_type() output, which holds no whitespace", + ( + "fluid_build/iac/providers/gcp.py", + "_bq_outcome", + r"^\((\d+)(?:,(\d+))?\)$", + ): "the value is column_types.split_type() output, which holds no whitespace", + ( + "fluid_build/iac/providers/databricks.py", + "_scale_zero_warnings", + r"^decimal(\([0-9]+\))?$", + ): "the value is dbt_types.sql_type() output", + ( + "fluid_build/iac/providers/databricks.py", + "_type_problem", + r"^[a-z_]+(<[a-z_]+>)?(\([0-9]{1,4}(,[0-9]{1,4})?\))?$", + ): "the only caller passes dbt_types.sql_type() output", + # Refusals and detectors: a match rejects or labels the value, so `$` errs on + # the safe side here, and \Z would loosen them. + ( + "fluid_build/build_runners/hooks/dlp_scan.py", + "_classify_value", + r"^[^\s@]+@[^\s@]+\.[^\s@]+$", + ): "a PII detector: a match labels the value as an email", + ( + "fluid_build/build_runners/hooks/dlp_scan.py", + "_classify_value", + r"^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}$", + ): "a PII detector: a match labels the value as an IP address", + ( + "fluid_build/build_runners/hooks/dlp_scan.py", + "_classify_value", + r"^\d{3}-\d{2}-\d{4}$", + ): "a PII detector: a match labels the value as an SSN", + ( + "fluid_build/providers/aws/util/validation.py", + "ResourceValidator.validate_s3_bucket_name", + r"^\d+\.\d+\.\d+\.\d+$", + ): "refuses a bucket name shaped like an IP address", + ( + "fluid_build/providers/gcp/util/names.py", + "normalize_bucket_name", + r"^\d+\.\d+\.\d+\.\d+$", + ): "refuses a bucket name shaped like an IP address", + ( + "fluid_build/providers/gcp/util/names.py", + "validate_name", + r"^\d+\.\d+\.\d+\.\d+$", + ): "refuses a bucket name shaped like an IP address", + # Parsers: only the captured groups are used, and nothing is validated. + ( + "fluid_build/build_runners/_retention.py", + "parse_iso_duration", + r"^P(?!$)(?:(\d+)Y)?(?:(\d+)M)?(?:(\d+)W)?(?:(\d+)D)?(?:T(?:(\d+)H)?(?:(\d+)M)?(?:(\d+)S)?)?$", + ): "returns a timedelta built from the captured numbers; every caller keeps that", + ( + "fluid_build/build_runners/duckdb/runner.py", + "_canonical_cursor_text", + r"(?P.*?)(?PZ|[+-]\d{2}:?(?:\d{2})?)$", + ): "rebuilds a cursor timestamp from its captured parts and refuses nothing", + ( + "fluid_build/cli/version_cmd.py", + "_bundled_schema_versions", + r"^fluid-schema-(\d+\.\d+(?:\.\d+)?)\.json$", + ): "reads the version out of a bundled schema's file name", + ( + "fluid_build/iac/providers/databricks.py", + "_canonical_type", + r"^decimal\(([0-9]{1,4})(?:,([0-9]{1,4}))?\)$", + ): "spells decimal(p) as decimal(p,0) from the captured digits and refuses nothing", + ( + "fluid_build/iac/providers/databricks_retention.py", + "_part_selects", + r"^(@)?(\d*\+)?(.*?)(\+\d*)?$", + ): "the selectors are split on whitespace first, and only the captured parts are used", + ( + "fluid_build/providers/_duckdb_sandbox.py", + "DuckDBAllowlist.with_locations", + r"^(?P[a-z][a-z0-9+.-]*)://(?P[^/?#]+)(?P/[^?#]*)?$", + ): "the host and path classes admit \\n, so the groups take a final newline under \\Z too", + ( + "fluid_build/providers/_duckdb_sandbox.py", + "_remote", + r"^(?P[a-z][a-z0-9+.-]*)://(?P[^/?#]+)(?P/[^?#]*)?$", + ): "the host and path classes admit \\n, so the groups take a final newline under \\Z too", + ( + "fluid_build/providers/_duckdb_sandbox.py", + "is_remote_location", + r"^(?P[a-z][a-z0-9+.-]*)://(?P[^/?#]+)(?P/[^?#]*)?$", + ): "the host and path classes admit \\n, so the groups take a final newline under \\Z too", + ( + "fluid_build/schema_manager.py", + "FluidSchemaManager._load_bundled_schemas", + r"^fluid-schema-(\d+\.\d+(?:\.\d+)?)\.json$", + ): "reads the version out of a bundled schema's file name", + ( + "fluid_build/schema_manager.py", + "_discover_bundled_versions", + r"^fluid-schema-(\d+\.\d+(?:\.\d+)?)\.json$", + ): "reads the version out of a bundled schema's file name", + ( + "fluid_build/schema_manager.py", + "_schema_version_sort_key", + r"^(\d+)\.(\d+)(?:\.(\d+))?(?:-(.+))?$", + ): "turns the captured numbers into a sort key", + # Judgment calls, and a known gap left for a follow-up. + ( + "fluid_build/providers/snowflake/util/names.py", + "quote_identifier", + r"^[A-Z_][A-Z0-9_]*$", + ): ( + "decides quoting and refuses nothing: under \\Z, ORDERS\\n would be quoted as the " + "object 'ORDERS\\n'. The identifier validators upstream are the guard" + ), + ( + "fluid_build/cli/import_workflow/dbt.py", + "_recency_window", + r"^P(?!$)(\d+Y)?(\d+M)?(\d+W)?(\d+D)?(T(\d+H)?(\d+M)?(\d+S)?)?$", + ): ( + "follow-up: returns fluid_window verbatim, and the duration parsers downstream " + "accept a final newline, so \\Z would drop the window. Strip it and return the " + "stripped value, as sla._as_iso_duration does" + ), + # Patterns read at run time. + ( + "fluid_build/build_runners/hooks/quality_gate.py", + "_check", + "", + ): ( + "follow-up: a contract's regex gate runs with re.match, so '^[A-Z]{3}$' passes " + "'ABC\\n'. The hook is not wired into a live chain yet (see its module note); " + "settle match against fullmatch before it is" + ), + ( + "fluid_build/cli/forge_copilot_contract_helpers.py", + "apply_overlay_validator_rules", + "", + ): "an overlay rule searches the serialized contract for text, not a whole value", + ( + "fluid_build/cli/forge_copilot_contract_helpers.py", + "apply_overlay_validator_rules", + "", + ): "an overlay rule searches the serialized contract for text, not a whole value", + ( + "fluid_build/schema_manager.py", + "_locate_error", + "", + ): ( + "mirrors jsonschema, which applies patternProperties with re.search; anything " + "else would point at a different key from the one the validator rejected" + ), + ( + "fluid_build/schema_manager.py", + "_pattern_keyword", + "", + ): ( + "jsonschema's pattern keyword: _end_anchored() turns every $ into \\Z, and .search " + "is what JSON Schema itself runs" + ), + ( + "fluid_build/schema_manager.py", + "_pattern_keyword", + "", + ): "only picks the error message, after the \\Z check above has refused the value", +} + +#: The entries that cover more than one call, with the number read; every other entry +#: covers one. So a new call with a listed pattern in a listed function is reported. +_ACCEPTED_CALLS: Dict[Tuple[str, str, str], int] = { + ( + "fluid_build/providers/odcs/mappers/sla.py", + "_as_availability_pct", + r"^(100(\.0+)?|\d{2}(\.\d+)?|\d{1}\d(\.\d+)?)%$", + ): 2, +} + + +@pytest.fixture(scope="module") +def tree() -> Tuple[_Scan, List[Finding]]: + scanner = _Scan(_PACKAGE) + return scanner, scanner.findings() + + +def test_the_scan_reads_every_module_and_follows_patterns( + tree: Tuple[_Scan, List[Finding]], +) -> None: + """A module the scan could not read, or a walk that followed no pattern, would let + the gate pass without checking anything.""" + scanner, _ = tree + assert scanner.unreadable == [], "the scan could not read:\n " + "\n ".join( + scanner.unreadable + ) + assert scanner.calls_seen > 150, f"only {scanner.calls_seen} calls had a pattern to follow" + + +def test_no_validator_lets_a_final_newline_through(tree: Tuple[_Scan, List[Finding]]) -> None: + _, found = tree + new = [f for f in found if f.key not in _ACCEPTED] + assert not new, ( + "These calls run a pattern that ends in `$` with .match or .search, so a value " + "with a final newline passes. End the pattern in \\Z or call fullmatch. If the " + "`$` is deliberate, add the call to _ACCEPTED with the reason:\n " + + "\n ".join(f.show() for f in new) + ) + + +def test_every_accepted_entry_still_matches_a_call(tree: Tuple[_Scan, List[Finding]]) -> None: + _, found = tree + keys = {f.key for f in found} + stale = sorted(key for key in _ACCEPTED if key not in keys) + assert not stale, "These _ACCEPTED entries match no call any more. Remove them:\n " + ( + "\n ".join(map(repr, stale)) + ) + + +def test_every_accepted_entry_covers_the_calls_it_was_read_for( + tree: Tuple[_Scan, List[Finding]], +) -> None: + """A second call with the same pattern in the same function was not read when the + entry was written, so it must not pass on the entry's reason.""" + _, found = tree + counts = Counter(f.key for f in found if f.key in _ACCEPTED) + changed = sorted(key for key, n in counts.items() if n != _ACCEPTED_CALLS.get(key, 1)) + assert not changed, ( + "These _ACCEPTED entries now cover a different number of calls. Read the new " + "call; if its `$` is deliberate too, set the count in _ACCEPTED_CALLS:\n " + + "\n ".join(f.show() for key in changed for f in found if f.key == key) + ) + + +def test_every_accepted_entry_gives_its_reason() -> None: + assert all(reason.strip() for reason in _ACCEPTED.values()) + assert set(_ACCEPTED_CALLS) <= set(_ACCEPTED) + + +def test_fluid_build_imports_re_as_a_module() -> None: + """The scan reads ``re.match(...)``, not a bare ``match(...)`` from ``from re import``.""" + offenders = [] + for path in sorted(_PACKAGE.rglob("*.py")): + text = path.read_text(encoding="utf-8") + if "from re import" not in text and "from regex import" not in text: + continue + for node in ast.walk(ast.parse(text)): + if isinstance(node, ast.ImportFrom) and node.module in _REGEX_MODULES: + offenders.append(f"{path.relative_to(_PACKAGE.parent)}:{node.lineno}") + assert not offenders, ( + "Import the regex module itself (`import re`), so this gate can read the calls, " + "or teach the scan the new form:\n " + "\n ".join(offenders) + ) + + +# ── the scan itself ───────────────────────────────────────────────────── + + +def _scan(tmp_path: pathlib.Path, **modules: str) -> List[str]: + """Scan a throwaway ``fluid_build`` package; one ``function: pattern`` line per report. + + A module name may name a subpackage: ``pkg__init__`` becomes ``pkg/__init__.py``.""" + package = tmp_path / "fluid_build" + package.mkdir() + (package / "__init__.py").write_text("", encoding="utf-8") + for name, source in modules.items(): + path = package / f"{name.replace('__init__', '/__init__')}.py" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(textwrap.dedent(source), encoding="utf-8") + scanner = _Scan(package) + found = scanner.findings() + assert scanner.unreadable == [] + return [f"{f.function}: {f.pattern}" for f in found] + + +@pytest.mark.parametrize( + "source, expected", + [ + pytest.param( + """ + import re + _NAME_RE = re.compile(r"^[a-z]+$") + + def check(value): + if not _NAME_RE.match(value): + raise ValueError(value) + """, + ["check: ^[a-z]+$"], + id="a-new-validator", + ), + pytest.param( + """ + import re + + def check(value): + return re.match(r"^[a-z]+$", value) + """, + ["check: ^[a-z]+$"], + id="re-match", + ), + pytest.param( + """ + import re as _re + + def check(value): + return _re.search(r"\\A[a-z]+$", value) + """, + [r"check: \A[a-z]+$"], + id="re-search-anchored", + ), + pytest.param( + """ + import regex + + def check(value): + return regex.match(r"^[a-z]+$", value) + """, + ["check: ^[a-z]+$"], + id="the-regex-module", + ), + pytest.param( + """ + import re + + def check(value): + return re.match(r"[a-z]+$", value) + """, + ["check: [a-z]+$"], + id="match-anchors-the-start-itself", + ), + pytest.param( + """ + import re + NAME = r"^[a-z]+$" + + def check(value): + return re.match(NAME, value) + """, + ["check: ^[a-z]+$"], + id="string-constant", + ), + pytest.param( + """ + import re + _PATTERNS = {"table": re.compile(r"^[a-z_]+$"), "project": re.compile(r"^[a-z-]+$")} + + def check(kind, value): + pattern = _PATTERNS.get(kind) + return pattern is not None and pattern.match(value) + """, + ["check: ^[a-z-]+$", "check: ^[a-z_]+$"], + id="dict-get", + ), + pytest.param( + """ + import re + _PATTERNS = {"table": re.compile(r"^[a-z_]+$"), "project": re.compile(r"^[a-z-]+\\Z")} + + def check(value): + return _PATTERNS["table"].match(value) + """, + ["check: ^[a-z_]+$"], + id="dict-key", + ), + pytest.param( + """ + import re + _RAW = {"table": r"^[a-z_]+$"} + _PATTERNS = {kind: re.compile(raw) for kind, raw in _RAW.items()} + + def check(kind, value): + return _PATTERNS[kind].match(value) + """, + ["check: ^[a-z_]+$"], + id="dict-comprehension", + ), + pytest.param( + """ + import re + from types import MappingProxyType + _PATTERNS = MappingProxyType({"table": re.compile(r"^[a-z_]+$")}) + + def check(kind, value): + return _PATTERNS[kind].match(value) + """, + ["check: ^[a-z_]+$"], + id="mapping-proxy", + ), + pytest.param( + """ + import re + _PATTERNS = dict(table=re.compile(r"^[a-z_]+$")) + + def check(kind, value): + return _PATTERNS[kind].match(value) + """, + ["check: ^[a-z_]+$"], + id="dict-keywords", + ), + pytest.param( + """ + import re + _DENY = (re.compile(r"^admin$"), re.compile(r"^root$")) + + def refused(value): + return any(rx.match(value) for rx in _DENY) + """, + ["refused: ^admin$", "refused: ^root$"], + id="tuple-in-a-comprehension", + ), + pytest.param( + """ + import re + _DENY = tuple([re.compile(r"^admin$")]) + + def refused(value): + for index, rx in enumerate(sorted(_DENY)): + if rx.match(value): + return index + """, + ["refused: ^admin$"], + id="tuple-sorted-enumerate", + ), + pytest.param( + """ + import re + _FIRST = (re.compile(r"^admin$"),) + _ALL = (*_FIRST, re.compile(r"^root\\Z")) + + def refused(value): + return _ALL[1].match(value) + """, + ["refused: ^admin$"], + id="starred-tuple", + ), + pytest.param( + """ + import re + _KINDS = {"email": re.compile(r"^[a-z]+@[a-z]+$")} + + def kinds(value): + for name, rx in _KINDS.items(): + if rx.match(value): + yield name + """, + ["kinds: ^[a-z]+@[a-z]+$"], + id="dict-items-loop", + ), + pytest.param( + """ + import functools + import re + + @functools.lru_cache(maxsize=None) + def _pattern(): + return re.compile(r"^[a-z]+$") + + def check(value): + return _pattern().match(value) + """, + ["check: ^[a-z]+$"], + id="helper-return", + ), + pytest.param( + """ + import re + _pattern = lambda: re.compile(r"^[a-z]+$") + + def check(value): + return _pattern().match(value) + """, + ["check: ^[a-z]+$"], + id="lambda-return", + ), + pytest.param( + """ + import re + + class Validator: + NAME = re.compile(r"^[a-z]+$") + + def check(self, value): + return self.NAME.match(value) + """, + ["Validator.check: ^[a-z]+$"], + id="class-attribute", + ), + pytest.param( + """ + import re + + class Validator: + def __init__(self): + self._name = re.compile(r"^[a-z]+$") + + def check(self, value): + return self._name.match(value) + """, + ["Validator.check: ^[a-z]+$"], + id="instance-attribute", + ), + pytest.param( + """ + import functools + import re + + class Validator: + @functools.cached_property + def name(self): + return re.compile(r"^[a-z]+$") + + def check(self, value): + return self.name.match(value) + """, + ["Validator.check: ^[a-z]+$"], + id="cached-property", + ), + pytest.param( + """ + import re + from typing import Generic, TypeVar + + T = TypeVar("T") + + class Base(Generic[T]): + NAME = re.compile(r"^[a-z]+$") + + class Child(Base[int]): + def check(self, value): + return self.NAME.match(value) or type(self).NAME.match(value) + """, + ["Child.check: ^[a-z]+$", "Child.check: ^[a-z]+$"], + id="generic-base-and-type-of-self", + ), + pytest.param( + """ + import re + + class Validator: + NAME = re.compile(r"^[a-z]+$") + + def check(self, value): + return self.__class__.NAME.match(value) + """, + ["Validator.check: ^[a-z]+$"], + id="dunder-class", + ), + pytest.param( + """ + import re + _NAME_RE = re.compile(r"^[a-z]+$") + _AGAIN_RE = re.compile(_NAME_RE.pattern, re.IGNORECASE) + + def check(value): + return _AGAIN_RE.match(value) + """, + ["check: ^[a-z]+$"], + id="pattern-attribute", + ), + pytest.param( + """ + import re + _NAME_RE = re.compile(r"^[a-z]+$") + + def names(values): + return list(filter(_NAME_RE.match, values)) + """, + ["names: ^[a-z]+$"], + id="method-reference", + ), + pytest.param( + """ + import re + + def check(prefix, value): + return re.match(rf"^{re.escape(prefix)}-[0-9]+$", value) + """, + ["check: ^…-[0-9]+$"], + id="f-string", + ), + pytest.param( + """ + import re + + def check(value): + return re.match("|".join([r"^[a-z]+$", r"^[0-9]+\\Z"]), value) + """, + ["check: ^[a-z]+$"], + id="joined-alternatives", + ), + pytest.param( + """ + import re + + def check(value): + return re.compile(r"^[a-z]+$|^[0-9]+$").match(value) + """, + ["check: ^[a-z]+$|^[0-9]+$"], + id="alternation", + ), + pytest.param( + """ + import re + + def check(value): + return re.search(r"^[a-z]+$|^[0-9]+\\Z", value) + """, + [r"check: ^[a-z]+$|^[0-9]+\Z"], + id="half-fixed-alternation", + ), + pytest.param( + """ + import re + + def check(value): + return re.search(r"^[a-z]+$|[0-9]+", value) + """, + ["check: ^[a-z]+$|[0-9]+"], + id="anchored-alternative-beside-an-unanchored-one", + ), + pytest.param( + """ + import re + + def check(value): + return re.search(r"(?:^[a-z]+$)", value) or re.match(r"^(?:[a-z]+$|[0-9]+)", value) + """, + ["check: (?:^[a-z]+$)", "check: ^(?:[a-z]+$|[0-9]+)"], + id="dollar-inside-a-group", + ), + pytest.param( + """ + import re + + def check(value): + return re.match(r"^ [a-z]+ $ # the name, and nothing after it", value, re.X) + """, + ["check: ^ [a-z]+ $ # the name, and nothing after it"], + id="verbose", + ), + pytest.param( + """ + import re + + def check(value): + return re.match(r"^a\\\\s*$", value) + """, + [r"check: ^a\\s*$"], + id="escaped-backslash-before-s-star", + ), + pytest.param( + """ + import re + + def check(value): + return re.compile(r"(?i)^[a-z]+$").match(value) + """, + ["check: (?i)^[a-z]+$"], + id="inline-flags", + ), + pytest.param( + """ + import re + _FLAGS = re.MULTILINE + _FLAGS = _FLAGS | re.IGNORECASE + + def check(value): + return re.search(r"^[a-z]+$", value, _FLAGS) + """, + ["check: ^[a-z]+$"], + id="flags-the-scan-cannot-settle-count-as-none", + ), + pytest.param( + """ + import re + if re.match(r"^v[0-9]+$", "v1"): + pass + """, + [": ^v[0-9]+$"], + id="module-level-call", + ), + pytest.param( + """ + import re + + def check(schema, value): + return re.compile(schema["pattern"]).match(value) + """, + ["check: "], + id="schema-pattern-at-run-time", + ), + pytest.param( + """ + import functools + import re + + @functools.lru_cache(maxsize=None) + def _semver_pattern(schema_text): + return re.compile(schema_text) + + def is_semver(value, schema_text): + return bool(_semver_pattern(schema_text).match(value)) + """, + ["is_semver: "], + id="schema-pattern-from-a-helper", + ), + pytest.param( + """ + import re + + def check(rule, value): + return re.search(rule["regex"], value) + """, + ["check: "], + id="re-search-at-run-time", + ), + pytest.param( + """ + import re + + def check(suffix, value): + return re.compile(f"{suffix}").match(value) + """, + ["check: "], + id="f-string-with-an-unknown-end", + ), + pytest.param( + """ + import re + from typing import Optional + + _DEFAULT = re.compile(r"^[A-Za-z_]+\\Z") + + def check(value, pattern: Optional[re.Pattern[str]] = None): + rx = pattern or _DEFAULT + return rx.match(value) + """, + ["check: "], + id="caller-supplied-pattern", + ), + pytest.param( + """ + from typing import Pattern as P + + def check(value, rx: "P[str]"): + return rx.search(value) + """, + ["check: "], + id="pattern-annotation-alias", + ), + ], +) +def test_the_scan_reports(tmp_path: pathlib.Path, source: str, expected: List[str]) -> None: + assert _scan(tmp_path, check=source) == expected + + +@pytest.mark.parametrize( + "source", + [ + pytest.param('_NAME_RE = re.compile(r"^[a-z]+\\Z")\n_NAME_RE.match(v)', id="backslash-z"), + pytest.param('re.match(r"^[a-z]+\\Z|^[0-9]+\\Z", v)', id="alternation-fixed"), + pytest.param('_NAME_RE = re.compile(r"^[a-z]+$")\n_NAME_RE.fullmatch(v)', id="fullmatch"), + pytest.param('re.fullmatch(r"[a-z]+$", v)', id="re-fullmatch"), + pytest.param('re.findall(r"^[a-z]+$", v)', id="findall"), + pytest.param('re.compile(r"^[a-z]+$", re.MULTILINE | re.I).search(v)', id="multiline"), + pytest.param('_FLAGS = re.M\nre.search(r"^[a-z]+$", v, _FLAGS)', id="multiline-constant"), + pytest.param('re.search(r"(?m)^[a-z]+$", v)', id="inline-multiline"), + pytest.param('re.search(r"(?m:^[a-z]+$)", v)', id="scoped-multiline"), + pytest.param('re.match(r"^key:\\s*$", v)', id="whitespace-tail"), + pytest.param('re.match(r"^[a-z]+://[^/]*$", v)', id="negated-class-tail"), + pytest.param('re.match(r"(?is)^\\s*distinct\\s+(.+)$", v)', id="dotall-tail-in-a-group"), + pytest.param('re.match(r"^(?:a\\s*|b\\s+)$", v)', id="every-branch-absorbs"), + pytest.param('re.search(r"\\.ya?ml$", v)', id="unanchored-suffix-search"), + pytest.param('re.match(r"^[0-9]+\\$", v)', id="escaped-dollar"), + pytest.param('re.match(r"^[a-z]+", v)', id="no-end-anchor"), + pytest.param( + 'words = ["a", "b"]\nre.compile("|".join(re.escape(w) for w in words)).search(v)', + id="escaped-alternatives", + ), + pytest.param( + 'import pathlib\npathlib.PurePath(v).match("*.py")\nstore.search("ns", v)', + id="not-a-regex", + ), + pytest.param( + "class GlobPattern:\n pass\n\ndef inner(p: GlobPattern):\n return p.match(v)", + id="an-annotation-that-only-contains-the-word", + ), + ], +) +def test_the_scan_does_not_report(tmp_path: pathlib.Path, source: str) -> None: + body = textwrap.indent(source, " ") + assert _scan(tmp_path, check=f"import re\n\ndef check(v, store):\n{body}\n") == [] + + +@pytest.mark.parametrize( + "importer", + [ + pytest.param("from .consts import NAME_RE\n", id="relative-import"), + pytest.param("from fluid_build.consts import NAME_RE\n", id="absolute-import"), + pytest.param("from fluid_build import consts\nNAME_RE = consts.NAME_RE\n", id="module"), + pytest.param("import fluid_build.consts as c\nNAME_RE = c.NAME_RE\n", id="import-as"), + ], +) +def test_the_scan_follows_a_pattern_across_modules(tmp_path: pathlib.Path, importer: str) -> None: + consts = 'import re\nNAME_RE = re.compile(r"^[a-z]+$")\n' + check = importer + "\ndef check(value):\n return NAME_RE.match(value)\n" + assert _scan(tmp_path, consts=consts, check=check) == ["check: ^[a-z]+$"] + + +def test_the_scan_follows_a_submodule_its_package_imports(tmp_path: pathlib.Path) -> None: + """``from . import consts`` in ``pkg/__init__.py`` binds the submodule itself.""" + found = _scan( + tmp_path, + pkg__init__="from . import consts\n", + **{"pkg/consts": 'import re\nNAME_RE = re.compile(r"^[a-z]+$")\n'}, + check=( + "from fluid_build.pkg import consts\n\n" + "def check(value):\n return consts.NAME_RE.match(value)\n" + ), + ) + assert found == ["check: ^[a-z]+$"] + + +@pytest.mark.timeout(60) +def test_the_scan_reads_long_chains_in_linear_time(tmp_path: pathlib.Path) -> None: + """Reading a receiver twice per call, or a name again at each use, doubles the work at + every step, and 40 steps would never finish. A string doubled 40 times is cut short.""" + reused = "\n".join(f"X{i + 1} = X{i} or X{i}" for i in range(40)) + doubled = "\n".join(f"Y{i + 1} = Y{i} + Y{i}" for i in range(40)) + source = ( + f'import re\nX0 = re.compile(r"^[a-z]+$")\nY0 = "a"\n{reused}\n{doubled}\n\n' + "def check(v):\n" + f" return v{'.strip()' * 40}.match(v) or X40.match(v) or re.match(Y40, v)\n" + ) + assert _scan(tmp_path, check=source) == ["check: ^[a-z]+$"] + + +def test_a_module_the_scan_cannot_read_is_listed(tmp_path: pathlib.Path) -> None: + package = tmp_path / "fluid_build" + package.mkdir() + (package / "__init__.py").write_text("", encoding="utf-8") + (package / "broken.py").write_text("def f(:\n RX.match(v)\n", encoding="utf-8") + (package / "latin1.py").write_bytes(b"# caf\xe9\nRX.match(v)\n") + deep = " + ".join(['"a"'] * 3000) + (package / "deep.py").write_text(f"import re\nre.match({deep}, v)\n", encoding="utf-8") + scanner = _Scan(package) + assert scanner.findings() == [] + listed = sorted(entry.split(":")[0] for entry in scanner.unreadable) + assert listed == ["fluid_build/broken.py", "fluid_build/deep.py", "fluid_build/latin1.py"] diff --git a/tests/test_litellm_endpoint_names.py b/tests/test_litellm_endpoint_names.py index c8b22d4a0..ee86591c4 100644 --- a/tests/test_litellm_endpoint_names.py +++ b/tests/test_litellm_endpoint_names.py @@ -80,6 +80,18 @@ def test_a_project_s_own_api_base_loads_with_its_own_key(tmp_path: Path) -> None assert os.environ["MYAPP_API_KEY"] == PROJECT_KEY +def test_databricks_api_base_is_refused_while_the_operator_holds_the_m2m_secret() -> None: + # litellm's Databricks OAuth M2M path takes the workspace from + # DATABRICKS_API_BASE and posts DATABRICKS_CLIENT_SECRET there + # (litellm 1.108.0 llms/databricks/common_utils.py), whatever key pairs it. + refused = unpaired_endpoints( + {"DATABRICKS_API_BASE": "https://evil.test", "FLUID_LLM_API_KEY": PROJECT_KEY}, + {"DATABRICKS_CLIENT_SECRET": OPERATOR_KEY}, + ) + + assert "DATABRICKS_CLIENT_SECRET" in refused["DATABRICKS_API_BASE"].operator_credentials + + def test_a_project_api_base_still_follows_the_name_shape_rule() -> None: # The operator's own MYAPP_API_KEY would go to the project's host. refused = unpaired_endpoints( @@ -102,10 +114,13 @@ def test_a_project_api_base_still_follows_the_name_shape_rule() -> None: "TENSORMESH_SERVERLESS_BASE_URL", "ANTHROPIC_AWS_BASE_URL", "COMETAPI_BASE_URL", + # a provider's route for one kind of call + "DASHSCOPE_API_BASE_VIDEO", + "QWENCLOUD_API_BASE_RERANK", ], ) def test_a_litellm_provider_endpoint_pairs_only_with_fluid_llm_api_key(endpoint: str) -> None: - prefix = re.sub(r"_(API_BASE|BASE_URL)$", "", endpoint) + prefix = re.sub(r"_(API_BASE(?:_[A-Z0-9]+)*|BASE_URL)$", "", endpoint) decoy = {endpoint: "https://llm.project.test", f"{prefix}_API_KEY": PROJECT_KEY} paired = {endpoint: "https://llm.project.test", "FLUID_LLM_API_KEY": PROJECT_KEY} @@ -237,7 +252,9 @@ def test_hydration_does_not_import_litellm(tmp_path: Path) -> None: "rust_bridge/", "router_strategy/", ) -_READ = re.compile(r"""["']([A-Z][A-Z0-9_]*_API_BASE)["']""") +# ``_API_BASE``, and the same with a kind after it (``DASHSCOPE_API_BASE_VIDEO``, +# ``QWENCLOUD_API_BASE_RERANK``): one provider's route for one kind of call. +_READ = re.compile(r"""["']([A-Z][A-Z0-9_]*_API_BASE(?:_[A-Z0-9]+)*)["']""") # A get_secret* / os.getenv / os.environ read of a name ending like a host, a # URL, an endpoint or an account id (the host Snowflake's is part of). _ENV_READ = re.compile( diff --git a/tests/test_welcome_scan_budget.py b/tests/test_welcome_scan_budget.py index d194a4c02..572a622f1 100644 --- a/tests/test_welcome_scan_budget.py +++ b/tests/test_welcome_scan_budget.py @@ -14,13 +14,15 @@ """Welcome-scan budget semantics. -Two promises, both measured against a real clock rather than a mocked one: +Three promises, all measured against a real clock rather than a mocked one: * the cold imports the probes trigger are paid before the budget starts, so the first scan of a process returns what a warm scan returns (on a loaded machine the AI probe's pydantic import used to blow the old per-probe window); * ``_TOTAL_TIMEOUT_S`` is a real cap: the scan returns on time and a probe it - gave up on cannot keep the process alive. + gave up on cannot keep the process alive; +* the one probe that can run long, the tree walk, ends when its scan does, and a + process holds at most one walker however many scans it runs. """ from __future__ import annotations @@ -141,6 +143,144 @@ def test_abandoned_probe_does_not_keep_the_process_alive(tmp_path): ) +# --------------------------------------------------------------------------- +# The tree walk ends with its scan +# --------------------------------------------------------------------------- + + +def _walkers(): + return [t for t in threading.enumerate() if t.name.startswith("welcome-scan-specialization")] + + +def _wait_until(predicate, timeout): + deadline = time.monotonic() + timeout + while not predicate(): + if time.monotonic() >= deadline: + return False + time.sleep(0.01) + return True + + +def _fan_tree(root: Path, fan: int = 20) -> int: + """``fan`` directories under ``root`` holding ``fan`` empty ones each; no contract anywhere. + + Returns how many directories a complete walk would list. + """ + for i in range(fan): + for j in range(fan): + (root / f"d{i:02d}" / f"e{j:02d}").mkdir(parents=True) + return 1 + fan + fan * fan + + +class _Listing: + """``os.scandir`` for the walker thread only; every other caller gets the real one. + + Each directory the walker lists waits ``delay`` seconds first. With ``hang`` the + first listing waits for ``release`` instead, as a call stuck on a dead network + mount would. ``release`` cuts every wait short, so cleanup is instant. + """ + + def __init__(self, delay: float = 0.0, hang: bool = False): + self.delay, self.hang = delay, hang + self.release = threading.Event() + self.listed: list = [] + self._real = os.scandir + + def __call__(self, path="."): + if threading.current_thread().name.startswith("welcome-scan-specialization"): + self.listed.append(str(path)) + self.release.wait(60 if self.hang and len(self.listed) == 1 else self.delay) + return self._real(path) + + +@pytest.fixture +def short_scan(monkeypatch): + """A 0.3s scan with no dbt and the imports paid: it returns at the cap while a walk is going.""" + monkeypatch.setattr(ws, "_TOTAL_TIMEOUT_S", 0.3) + monkeypatch.setattr( + "fluid_build.build_runners.dbt.runner._resolve_dbt_executable", lambda: None + ) + ws._warm_probe_imports() + assert _wait_until(lambda: not _walkers(), 2.0), "a walker from an earlier test is still alive" + + +def test_a_walk_the_scan_gave_up_on_ends_within_a_second_of_the_scan( + tmp_path, monkeypatch, short_scan +): + """Python cannot kill the walker, so the scan tells it to stop and it ends at its next directory. + + The tree takes 21s to walk at 50 ms a directory and the scan gives up after 0.3s. + Without the stop flag the walker reads on for the other 20s. + """ + directories = _fan_tree(tmp_path) + stand_in = _Listing(delay=0.05) + monkeypatch.setattr(os, "scandir", stand_in) + try: + ws.run_welcome_scan(start=tmp_path) + returned = time.monotonic() + gone = _wait_until(lambda: not _walkers(), timeout=2.0) + ended_after = time.monotonic() - returned + listed = len(stand_in.listed) + time.sleep(0.3) + assert gone, f"the walker was still reading {ended_after:.1f}s after the scan returned" + assert len(stand_in.listed) == listed # not one more directory once it ended + assert listed < directories / 4 # nowhere near the whole tree + finally: + stand_in.release.set() + + +def test_consecutive_scans_never_have_two_walks_reading_at_once(tmp_path, monkeypatch, short_scan): + """Each scan abandons its walk; the next scan's walk waits for it instead of overlapping.""" + _fan_tree(tmp_path) + stand_in = _Listing(delay=0.05) + monkeypatch.setattr(os, "scandir", stand_in) + reading = {"now": 0, "most": 0, "walks": 0} + lock = threading.Lock() + real_walk = ws._walk_contracts + + def counted(start, stop): + with lock: + reading["now"] += 1 + reading["walks"] += 1 + reading["most"] = max(reading["most"], reading["now"]) + try: + return real_walk(start, stop) + finally: + with lock: + reading["now"] -= 1 + + monkeypatch.setattr(ws, "_walk_contracts", counted) + try: + for _ in range(3): + ws.run_welcome_scan(start=tmp_path) + assert _wait_until(lambda: len(_walkers()) <= 1, timeout=1.0) + assert _wait_until(lambda: not _walkers(), timeout=2.0) # none outlives the last scan + finally: + stand_in.release.set() + + assert reading["walks"] >= 1 + assert reading["most"] == 1 + + +def test_three_scans_over_a_hung_filesystem_leave_one_walker(tmp_path, monkeypatch, short_scan): + """A walk stuck inside a filesystem call (a dead network mount) cannot be stopped. + + The scans after it must not add a stuck thread each: their walks wait behind the + first one and leave with their own scan. Before the gate: three walkers. + """ + _fan_tree(tmp_path, fan=2) + stand_in = _Listing(hang=True) + monkeypatch.setattr(os, "scandir", stand_in) + try: + for _ in range(3): + ws.run_welcome_scan(start=tmp_path) + assert _wait_until(lambda: len(_walkers()) == 1, timeout=2.0), _walkers() + assert len(stand_in.listed) == 1 # only the first walker ever reached the filesystem + finally: + stand_in.release.set() + assert _wait_until(lambda: not _walkers(), timeout=2.0) # unstuck, it sees its stop and ends + + # --------------------------------------------------------------------------- # Cold imports are off the clock # --------------------------------------------------------------------------- diff --git a/tests/test_welcome_scan_walk.py b/tests/test_welcome_scan_walk.py new file mode 100644 index 000000000..22d40a61c --- /dev/null +++ b/tests/test_welcome_scan_walk.py @@ -0,0 +1,316 @@ +# Copyright 2024-2026 Agentics Transformation Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""The tree walk behind the welcome scan's specialization probe. + +``_find_contracts`` replaced ``sorted(start.rglob("contract.fluid.yaml"))[:8]``, which +read the whole tree (279 s under a directory holding many repos). It has to return the +same contracts in the same order wherever its bounds do not cut the walk, and stop +where they do. +""" + +from __future__ import annotations + +import os +import sys +import threading +import time +from pathlib import Path + +import pytest + +from fluid_build.cli import _welcome_scan as ws +from fluid_build.util.upstream_discovery import BUILD_OUTPUT_DIRS, IGNORED_DIRS + +pytestmark = pytest.mark.unit + +_BODY = "metadata:\n productType: SDP\n" +_posix_only = pytest.mark.skipif(sys.platform == "win32", reason="needs symlinks / FIFOs") + + +def _contract(root: Path, *parts: str, body: str = _BODY) -> Path: + directory = root.joinpath(*parts) + directory.mkdir(parents=True, exist_ok=True) + path = directory / "contract.fluid.yaml" + path.write_text(body) + return path + + +@pytest.fixture +def listings(tmp_path, monkeypatch): + """The directories under ``tmp_path`` that the walk hands to ``os.scandir``, in order.""" + seen = [] + real = os.scandir + + def spy(path="."): + if str(path).startswith(str(tmp_path)): + seen.append(Path(path)) + return real(path) + + monkeypatch.setattr(os, "scandir", spy) + return seen + + +# --------------------------------------------------------------------------- +# Same contracts, same order as rglob +# --------------------------------------------------------------------------- + + +def test_the_contracts_come_out_as_rglob_lists_them(tmp_path): + """Depth-first in name order is ``sorted(rglob(...))``, so the first 8 are unchanged. + + The names are the ones where a plain string sort of the full paths and a path + comparison disagree ("a.b" sorts before "a/x" as text, after it as components), and + a contract file falls between its sibling directories ("b" < "contract..." < "z"). + """ + for parts in [ + ("a",), + ("a", "b"), + ("a", "z"), + ("a.b",), + ("a-b",), + ("B",), + ("b", "c"), + ("m",), + ("m", "n"), + ("m", "n", "o"), + ("x y",), + (".hidden",), + ("z",), + ]: + _contract(tmp_path, *parts) + _contract(tmp_path) # the start directory's own + + expected = sorted(tmp_path.rglob("contract.fluid.yaml")) + assert len(expected) > ws._SPECIALIZATION_SAMPLE # the cut at 8 is exercised + assert ws._find_contracts(tmp_path) == expected[: ws._SPECIALIZATION_SAMPLE] + + +def test_fewer_than_the_sample_are_all_returned(tmp_path): + for name in ("p1", "p2", "p3"): + _contract(tmp_path, name) + assert ws._find_contracts(tmp_path) == sorted(tmp_path.rglob("contract.fluid.yaml")) + + +def test_the_walk_ends_at_the_last_contract_it_needs(tmp_path, listings): + """Twenty products, eight wanted: the other twelve directories are never read.""" + for i in range(20): + _contract(tmp_path, f"p{i:02d}") + + found = ws._find_contracts(tmp_path) + + assert [p.parent.name for p in found] == [f"p{i:02d}" for i in range(8)] + assert listings == [tmp_path] + [tmp_path / f"p{i:02d}" for i in range(8)] + + +def test_a_missing_start_has_no_contracts(tmp_path): + assert ws._find_contracts(tmp_path / "nowhere") == [] + + +# --------------------------------------------------------------------------- +# What it does not enter +# --------------------------------------------------------------------------- + + +def test_the_repos_skip_list_is_not_entered(tmp_path): + """A venv holds a copy of every shipped template; a build tree holds copies of contracts. + + Every name on the repo's list (``upstream_discovery``), contract directly inside + and one level down, and none of them is tallied. + """ + skipped = sorted(IGNORED_DIRS | BUILD_OUTPUT_DIRS) + assert {".git", ".venv", "venv", "site-packages", "node_modules", "__pycache__"} <= set(skipped) + assert {"build", "dist", "target"} <= set(skipped) + for name in skipped: + _contract(tmp_path, name) + _contract(tmp_path, name, "pkg") + kept = _contract(tmp_path, "products", "orders") + + assert ws._find_contracts(tmp_path) == [kept] + + +def test_directories_below_the_depth_cap_are_not_listed(tmp_path): + cap = ws._SPECIALIZATION_MAX_DEPTH + levels = [f"l{i}" for i in range(cap + 1)] + at_cap = _contract(tmp_path, *levels[:cap]) # `cap` levels below the start: listed + _contract(tmp_path, *levels) # one more level: not listed + + assert ws._find_contracts(tmp_path) == [at_cap] + + +def test_entries_beyond_the_cap_are_not_read(tmp_path, monkeypatch, listings): + """Once the entry budget is spent the walk ends, wherever it is.""" + monkeypatch.setattr(ws, "_SPECIALIZATION_MAX_ENTRIES", 50) + (tmp_path / "a").mkdir() + for i in range(200): + (tmp_path / "a" / f"f{i:03d}.csv").write_text("x") + _contract(tmp_path, "b") # sorts after "a", but the budget is gone by then + + assert ws._find_contracts(tmp_path) == [] + assert listings == [tmp_path, tmp_path / "a"] # "a" read in part, "b" never listed + + +@_posix_only +def test_symlinks_and_special_files_are_not_tallied(tmp_path): + """``rglob`` would open a symlink to /dev/zero or a FIFO named like a contract, and block.""" + real = _contract(tmp_path, "real") + os.symlink(tmp_path / "real", tmp_path / "linked_dir") # not followed: no second copy + (tmp_path / "linked_file").mkdir() + os.symlink(real, tmp_path / "linked_file" / "contract.fluid.yaml") # not a regular file + (tmp_path / "fifo").mkdir() + os.mkfifo(tmp_path / "fifo" / "contract.fluid.yaml") # reading it would hang + + assert ws._find_contracts(tmp_path) == [real] + + +def test_an_unreadable_directory_is_skipped(tmp_path, monkeypatch): + locked = tmp_path / "a_locked" + locked.mkdir() + kept = _contract(tmp_path, "b_open") + real = os.scandir + + def scandir(path="."): + if Path(path) == locked: + raise PermissionError(13, "Permission denied", str(path)) + return real(path) + + monkeypatch.setattr(os, "scandir", scandir) + + assert ws._find_contracts(tmp_path) == [kept] + + +# --------------------------------------------------------------------------- +# The stop flag +# --------------------------------------------------------------------------- + + +def test_a_stop_flag_that_is_already_set_reads_nothing(tmp_path, listings): + _contract(tmp_path, "p") + stop = threading.Event() + stop.set() + + assert ws._find_contracts(tmp_path, stop) == [] + assert listings == [] + + +def test_the_stop_flag_is_checked_before_every_directory(tmp_path, monkeypatch): + for i in range(10): + _contract(tmp_path, f"p{i}") + stop = threading.Event() + read = [] + real = os.scandir + + def scandir(path="."): + if str(path).startswith(str(tmp_path)): + read.append(Path(path)) + if len(read) == 2: # the scan is over while the second directory is being read + stop.set() + return real(path) + + monkeypatch.setattr(os, "scandir", scandir) + + found = ws._find_contracts(tmp_path, stop) + + assert read == [tmp_path, tmp_path / "p0"] # the third directory is never read + assert found == [tmp_path / "p0" / "contract.fluid.yaml"] + + +def test_a_walk_waiting_for_the_gate_leaves_as_soon_as_its_scan_ends(tmp_path, monkeypatch): + """The budget is not what ends this wait: the scan's stop flag does, e.g. on Ctrl-C.""" + _contract(tmp_path, "p") + monkeypatch.setattr(ws, "_TOTAL_TIMEOUT_S", 60.0) + stop = threading.Event() + result = [] + worker = threading.Thread(target=lambda: result.append(ws._find_contracts(tmp_path, stop))) + worker.daemon = True + assert ws._WALK_GATE.acquire(timeout=5), "a walker from an earlier test holds the gate" + try: + worker.start() + time.sleep(0.15) + assert worker.is_alive() # it is waiting behind the held gate + stop.set() + worker.join(timeout=3) + assert not worker.is_alive(), "kept waiting after its scan ended" + assert result == [[]] + finally: + ws._WALK_GATE.release() + + +def test_a_walk_with_no_stop_flag_gives_up_on_a_held_gate_after_one_scan_budget( + tmp_path, monkeypatch +): + """A direct caller has no scan to end its wait; it must not poll a stuck walker forever. + + A worker thread, so a regression fails the assertion instead of hanging the run. + """ + _contract(tmp_path, "p") + monkeypatch.setattr(ws, "_TOTAL_TIMEOUT_S", 0.2) + result = [] + worker = threading.Thread(target=lambda: result.append(ws._find_contracts(tmp_path))) + worker.daemon = True + assert ws._WALK_GATE.acquire(timeout=5), "a walker from an earlier test holds the gate" + try: + worker.start() + worker.join(timeout=3) + assert not worker.is_alive(), "still waiting on the gate long after the scan budget" + assert result == [[]] + finally: + ws._WALK_GATE.release() + + +# --------------------------------------------------------------------------- +# The probe keeps its tally +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + ("types", "suggested"), + [ + (["SDP"] * 4, ""), # four typed contracts are too few + (["SDP"] * 4 + ["ADP"], "SDP"), # 4 of 5 + (["SDP"] * 3 + ["ADP"] * 2, ""), # 3 of 5 + (["SDP"] * 4 + ["ADP"] * 2 + ["CDP"] * 2, "SDP"), # 4 of 8: a count, not a ratio + (["SDP", None, None, None, None], ""), # untyped contracts do not count + # only the first 8 are tallied: the 4 SDP after them would make 7 + (["SDP"] * 3 + ["ADP"] * 3 + ["CDP"] * 2 + ["SDP"] * 4, ""), + ], +) +def test_the_tally_rule_is_unchanged(tmp_path, types, suggested): + for i, product_type in enumerate(types): + body = "id: x\n" if product_type is None else f"metadata:\n productType: {product_type}\n" + _contract(tmp_path, f"p{i:02d}", body=body) + + result = ws._probe_specialization_suggestion(tmp_path) + + assert result == ({"suggested_data_product_type": suggested} if suggested else {}) + + +def test_the_parse_loop_ends_when_the_scan_does(tmp_path, monkeypatch): + """A parse cannot be interrupted, so the probe looks at the flag before each contract.""" + import yaml + + for i in range(6): + _contract(tmp_path, f"p{i}") + stop = threading.Event() + parsed = [] + + def safe_load(text): + parsed.append(text) + stop.set() # the scan returns while the first contract is being parsed + return {"metadata": {"productType": "SDP"}} + + monkeypatch.setattr(yaml, "safe_load", safe_load) + + assert ws._probe_specialization_suggestion(tmp_path, stop) == {} + assert len(parsed) == 1