From fa8aa5e8baeffc8f6b983528bfa68101e1040ba8 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:54:56 +0200 Subject: [PATCH 1/6] fix(security): keep credentials out of connector config files, run records, log context and mission diagnostics Triage of the 20 open HIGH CodeQL alerts on main. Each was traced by an investigator and a skeptic and reproduced on 9482607a. Real exposures fixed: - #211 Debezium Server application.properties held the secretRef-resolved source password (0644 before a swallowed chmod, kept, not git-ignored). Credential-valued properties now render as ${FLUID_DBZ_} and the value goes to the server's environment (SmallRye Config resolves env, ordinal 300; the line stays per DBZ-2886). The file is created 0600 via _atomic_write_bytes (O_EXCL + rename). - #53/#54 Singer tap_config.json: written 0600 in a private 0700 TemporaryDirectory per tap call (the dbt/profiles.py pattern), removed on every exit path including timeout; a stale workdir copy is deleted. - #241 fluid mission quoted an inline contract credential in a failing predicate (terminal, --json, scorecard, LLM prompt). Leaves at or under a credential-named key now show as ***REDACTED***. - #231 the shared redactor missed aws.secret.access.key=, secret.access.key= and fs.s3a.secret.key=, so a Kafka Connect trace kept an S3 key in the run record. - #292 the local provider's redact_string now chains the shared redactor (postgres:// userinfo, token=, held literals in the log record ctx). Defence in depth: fluid init's .gitignore block ignores .fluid/debezium/ and .fluid/meltano/. False-positive class (11 alerts): CodeQL's SensitiveFunctionCall treats a call to a function named like *secret* as a source, so mask_known_secrets' output looked like a credential. Renamed to redact_known_values ("redact" is on CodeQL's notSensitiveRegexp, github/codeql shared/concepts/codeql/concepts/internal/SensitiveDataHeuristics.qll); the old name stays as an alias. The Debezium live test now renders through the real renderer, passes the credentials via env_file, and uses the pinned Silo image (minio/minio left Docker Hub on 2026-09-11). --- .secrets.baseline | 10 +- CHANGELOG.md | 40 ++ fluid_build/build_runners/debezium/runner.py | 93 +++- fluid_build/build_runners/meltano/runner.py | 100 +++-- fluid_build/cli/init.py | 4 + fluid_build/copilot/missions/checks.py | 42 +- fluid_build/observability/secret_redactor.py | 31 +- fluid_build/providers/local/util/logging.py | 8 +- .../providers/snowflake/util/logging.py | 8 +- tests/build_runners/test_catalog_auth_sink.py | 7 +- .../test_debezium_server_iceberg_live.py | 80 ++-- ...tials_stay_out_of_files_and_diagnostics.py | 418 ++++++++++++++++++ .../test_security_regressions_round3.py | 2 +- 13 files changed, 726 insertions(+), 117 deletions(-) create mode 100644 tests/security/test_credentials_stay_out_of_files_and_diagnostics.py diff --git a/.secrets.baseline b/.secrets.baseline index 39403351..e5013d79 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -427,7 +427,7 @@ "filename": "fluid_build/observability/secret_redactor.py", "hashed_secret": "1348b145fa1a555461c1b790a2f66614781091e9", "is_verified": false, - "line_number": 86 + "line_number": 156 } ], "fluid_build/providers/_sql_safety.py": [ @@ -470,7 +470,7 @@ "filename": "fluid_build/providers/local/util/logging.py", "hashed_secret": "9d4e1e23bd5b727046a9e3b4b7db57bd8d6ee684", "is_verified": false, - "line_number": 43 + "line_number": 45 } ], "fluid_build/providers/snowflake/types.py": [ @@ -488,14 +488,14 @@ "filename": "fluid_build/providers/snowflake/util/logging.py", "hashed_secret": "5baa61e4c9b93f3f0682250b6cf8331b7ee68fd8", "is_verified": false, - "line_number": 36 + "line_number": 42 }, { "type": "Private Key", "filename": "fluid_build/providers/snowflake/util/logging.py", "hashed_secret": "1348b145fa1a555461c1b790a2f66614781091e9", "is_verified": false, - "line_number": 129 + "line_number": 165 } ], "fluid_build/pypi/experimental/build-metadata/0.7.1.dev2.json": [ @@ -2154,5 +2154,5 @@ } ] }, - "generated_at": "2026-10-07T14:57:38Z" + "generated_at": "2026-10-13T19:51:44Z" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 43099db4..4094f777 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -113,6 +113,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 with `env:` or carries a `${...}` expression does not warn. Under `fluid validate --strict` the warning fails validation; use `sink.catalogAuth`, or an `env:` value. +- The embedded Debezium Server's `application.properties` no longer holds credentials. A + property whose name carries `password`, `secret`, `token`, `credential`, `jaas.config` + or `connection.string` is written as `${FLUID_DBZ_}` (for example + `debezium.source.database.password=${FLUID_DBZ_SOURCE_DATABASE_PASSWORD}`), and forge-cli + passes the value to the server in that environment variable. A `server_binary` wrapper + that replaces the environment must pass the `FLUID_DBZ_*` variables through. An + `env:` value is still written as it is. +- A Meltano / Singer tap now reads its `--config` file from a private temporary + directory that is removed when the tap exits, instead of + `.fluid/meltano///tap_config.json`. A copy an earlier release left there + is deleted on the next run. A tap that writes a refreshed token back to its config file + loses that write; the next run resolves the credential again. +- `fluid init` now git-ignores `.fluid/debezium/` and `.fluid/meltano/`. A project whose + `.gitignore` already has the fluid-cli block does not get the new lines; add them + yourself, and check that no `application.properties` or `tap_config.json` under those + directories was committed with a credential in it. ### Added @@ -414,6 +430,30 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Snowflake-managed expose removed or re-cataloged in this change) and keeps the `tofu state rm` remediation. +### Security +- **Credentials no longer reach disk in generated connector config.** The embedded Debezium + Server's `application.properties` held the source database password that a `secretRef` + resolved, under the contract directory, briefly at mode 0644 and kept after the run; the + Singer `tap_config.json` held it at 0644, was never deleted, and neither file was + git-ignored, so `git add -A` staged the password. The Debezium credential now goes to the + server's environment and the file is created at 0600; the tap config lives 0600 in a + private temporary directory for the life of the tap. (CodeQL alerts 211, 53, 54.) +- **`fluid mission` no longer quotes a contract's inline credential.** A failing predicate on + a path such as `exposes[*].binding.location.password` printed `got ''` to the + terminal, `--json`, the stored scorecard and the planner's LLM prompt, because the + redactor read `password: expected == …` as the assignment. A leaf at or under a + credential-named key now shows as `***REDACTED***`. (CodeQL alert 241.) +- **The shared redactor masks `aws.secret.access.key=`, `secret.access.key=` and + `fs.s3a.secret.key=`.** It knew only the `_` and `-` spellings, so a Kafka Connect task + trace stored in a run record kept an S3 secret key. (CodeQL alert 231.) +- **The local provider's log context is redacted by the shared redactor.** Its own + patterns missed `postgres://user:password@` URLs and `token=`, and a failed SQL action + put the raw error into the log record's context, which a JSON log formatter renders. + (CodeQL alert 292.) +- `mask_known_secrets` is now `redact_known_values`; the old name stays as an alias. CodeQL + treats a call to a function named like `*secret*` as a source of secrets, so every caller + of the redactor was reported as logging or storing a credential it had already masked. + ### Documentation - `docs/governance-parity.md` said these refusals happen at `fluid plan`. `fluid plan` diff --git a/fluid_build/build_runners/debezium/runner.py b/fluid_build/build_runners/debezium/runner.py index e107117f..4c69e8c0 100644 --- a/fluid_build/build_runners/debezium/runner.py +++ b/fluid_build/build_runners/debezium/runner.py @@ -29,12 +29,13 @@ import logging import os +import re import shutil import subprocess import time from dataclasses import dataclass from pathlib import Path -from typing import Any, ClassVar, Dict, FrozenSet, List, Mapping, Optional +from typing import Any, ClassVar, Dict, FrozenSet, List, Mapping, Optional, Tuple from fluid_build.api.runner import ( RunContext, @@ -490,6 +491,62 @@ def _escape_properties_value(value: str) -> str: return escaped +#: A property whose VALUE is a credential: Debezium's PASSWORD-typed fields +#: (``database.password``, ``*.ssl.*.password``), the Iceberg / S3 / JDBC / +#: GCS / ADLS secrets (``credential``, ``token``, ``s3.secret-access-key``, +#: ``s3.session-token``, ``jdbc.password``, ``adls.sas-token.*``), a Kafka JAAS +#: blob, and a Mongo connection string (it may carry userinfo). Narrower than +#: ``is_sensitive_key_name`` on purpose: ``rest.auth.type`` and +#: ``oauth2-server-uri`` are not secrets, and stay readable in the file. +_CREDENTIAL_PROPERTY_RE = re.compile( + r"(?i)password|secret|token|credential|jaas\.config|connection\.string" +) + + +def render_application_properties( + source_config: Mapping[str, Any], + sink_type: str, + sink_config: Mapping[str, Any], +) -> Tuple[str, Dict[str, str]]: + """Render Debezium Server's ``application.properties``, and the environment + that carries its credentials. + + A credential-valued property is written as a ``${FLUID_DBZ_}`` + expression, and its value goes into the returned environment, which the + caller passes to the server. Quarkus resolves the expression from the + environment (SmallRye Config: environment variables, ordinal 300, are a + config source like the file). The line stays in the file because Debezium + Server only collects the ``debezium.source.*`` properties it finds there + (DBZ-2886). So no credential is written to disk: the file sits under the + contract directory, where ``git add -A`` would stage it. + + An ``env:`` value is written as it is: it is Iceberg's own reference + to an environment variable (``sink.catalogAuth``), not a secret. + + Raises ``ValueError`` for a control character in a key or value + (:func:`_properties_line`), or for two properties whose names map to the + same environment variable. + """ + env: Dict[str, str] = {} + + def line(key: str, value: object) -> str: + rendered = _properties_line(key, value) + text = str(value) + if not text or text.startswith("env:") or not _CREDENTIAL_PROPERTY_RE.search(key): + return rendered + name = "FLUID_DBZ_" + re.sub(r"[^A-Za-z0-9]", "_", key.removeprefix("debezium.")).upper() + if env.get(name, text) != text: + raise ValueError(f"two debezium properties map to the environment variable {name}") + env[name] = text + return f"{_escape_properties_key(key)}=${{{name}}}" + + lines = [line(f"debezium.source.{k}", v) for k, v in source_config.items()] + lines.append("quarkus.log.console.json=false") + lines.append(_properties_line("debezium.sink.type", sink_type)) + lines += [line(f"debezium.sink.{sink_type}.{k}", v) for k, v in sink_config.items()] + return "\n".join(lines), env + + def _execute_debezium_server( ctx: RunContext, build: Mapping[str, Any], @@ -574,22 +631,20 @@ def _execute_debezium_server( # Render the line-based .properties file. Every contract-derived key/value # routes through _properties_line, which fail-closes on control characters so # a newline in a binding value cannot inject extra Debezium directives. + # Credentials go to the server's environment, not the file. try: - lines = [_properties_line(f"debezium.source.{k}", v) for k, v in source_config.items()] - lines.append("quarkus.log.console.json=false") - lines.append(_properties_line("debezium.sink.type", sink_type)) - for k, v in sink_config.items(): - lines.append(_properties_line(f"debezium.sink.{sink_type}.{k}", v)) + properties, secret_env = render_application_properties( + source_config, sink_type, sink_config + ) except ValueError as exc: return _failed(ctx, started_at, t_start, str(exc)) - config_path.write_text("\n".join(lines), encoding="utf-8") - # The file holds the source DB password and any operator-forwarded sink - # credentials (s3.secret-access-key / jdbc.password); it inherits the umask - # (0o644) otherwise. Force 0o600, mirroring dbt/profiles.py. - try: - os.chmod(config_path, 0o600) - except OSError: - pass + from ...credentials.encrypted_store import _atomic_write_bytes + + # Created at 0o600 (O_EXCL, then an atomic rename): never briefly + # world-readable, and no chmod whose failure could be ignored. A + # hand-written sink key the credential rule does not name still lands + # in this file. + _atomic_write_bytes(config_path, properties.encode("utf-8"), mode=0o600) # Validate any contract-supplied ``server_binary`` before it becomes # ``argv[0]`` (arbitrary-binary-execution guard). A value with a path @@ -607,13 +662,15 @@ def _execute_debezium_server( "(bare name on PATH, or an absolute path to an executable file)", ) - # No ``env=``: 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 secret is never written to application.properties. + # 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 + # ``FLUID_DBZ_*`` credentials the file refers to are added to it. No + # secret is written to application.properties. try: proc = subprocess.run( [binary, "--config", str(config_path)], + env={**os.environ, **secret_env} if secret_env else None, capture_output=True, text=True, timeout=int(dbz_props.get("timeout_seconds", 60)), diff --git a/fluid_build/build_runners/meltano/runner.py b/fluid_build/build_runners/meltano/runner.py index 5ff6a5fa..4053514b 100644 --- a/fluid_build/build_runners/meltano/runner.py +++ b/fluid_build/build_runners/meltano/runner.py @@ -33,11 +33,14 @@ import json import logging +import os import re import shutil import subprocess import sys +import tempfile import time +from contextlib import contextmanager from dataclasses import dataclass from pathlib import Path from typing import Any, ClassVar, Dict, FrozenSet, Iterator, List, Optional @@ -177,6 +180,31 @@ def _resolve_tap_binary(tap_name: str, *, project_dir: Optional[Path] = None) -> return None +@contextmanager +def _private_tap_config(config: Dict[str, Any], workdir: Path) -> Iterator[Path]: + """Yield the path of a tap's ``--config`` file that only this user can read. + + Singer's ``--config`` contract puts the resolved credential in a file: it + is how a tap reads its password. The file is created 0o600 inside a + 0o700 ``TemporaryDirectory`` (the pattern ``dbt/profiles.py`` uses for a + generated ``profiles.yml``), and removed when the tap exits, however it + exits. It used to be ``/tap_config.json``: under the contract + directory, 0o644, never deleted, and not in the ``.gitignore`` that + ``fluid init`` writes, so ``git add -A`` staged the password. A copy an + earlier release left there is deleted. + + A tap that writes a refreshed OAuth token back to its config file loses + that write; the next run resolves the credential from ``secretRef`` again. + """ + (workdir / "tap_config.json").unlink(missing_ok=True) + with tempfile.TemporaryDirectory(prefix="fluid-singer-") as tmp: + path = Path(tmp) / "tap_config.json" + fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + with os.fdopen(fd, "w", encoding="utf-8") as fh: + json.dump(config, fh) + yield path + + def invoke_tap( binary: str, *, @@ -197,29 +225,28 @@ def invoke_tap( workdir.mkdir(parents=True, exist_ok=True) # Resolve to absolute paths so the subprocess can find them # regardless of its cwd. Using relative paths here would have the - # tap look for ``//tap_config.json`` after subprocess + # tap look for ``//tap_state.json`` after subprocess # cd's into ``workdir`` — that's a double-prefix bug we hit. - config_path = (workdir / "tap_config.json").resolve() - config_path.write_text(json.dumps(config), encoding="utf-8") - cmd = [binary, "--config", str(config_path)] - if state is not None: - state_path = (workdir / "tap_state.json").resolve() - state_path.write_text(json.dumps(state), encoding="utf-8") - cmd += ["--state", str(state_path)] - if catalog is not None: - catalog_path = (workdir / "tap_catalog.json").resolve() - catalog_path.write_text(json.dumps(catalog), encoding="utf-8") - cmd += ["--catalog", str(catalog_path)] - - LOG.info("singer.invoke binary=%s cwd=%s", binary, workdir) - proc = subprocess.run( - cmd, - cwd=str(workdir.resolve()), - capture_output=True, - text=True, - timeout=timeout_seconds, - check=False, - ) + with _private_tap_config(config, workdir) as config_path: + cmd = [binary, "--config", str(config_path)] + if state is not None: + state_path = (workdir / "tap_state.json").resolve() + state_path.write_text(json.dumps(state), encoding="utf-8") + cmd += ["--state", str(state_path)] + if catalog is not None: + catalog_path = (workdir / "tap_catalog.json").resolve() + catalog_path.write_text(json.dumps(catalog), encoding="utf-8") + cmd += ["--catalog", str(catalog_path)] + + LOG.info("singer.invoke binary=%s cwd=%s", binary, workdir) + proc = subprocess.run( + cmd, + cwd=str(workdir.resolve()), + capture_output=True, + text=True, + timeout=timeout_seconds, + check=False, + ) return collect_singer_output(stream_singer_messages(iter(proc.stdout.splitlines()))) | { "exit_code": proc.returncode, "stderr": proc.stderr, @@ -243,23 +270,20 @@ def discover_tap_catalog( ``invoke_tap`` call). """ workdir.mkdir(parents=True, exist_ok=True) - # See note in invoke_tap — paths must be absolute so the tap finds - # them after subprocess.run cd's into workdir. - config_path = (workdir / "tap_config.json").resolve() - config_path.write_text(json.dumps(config), encoding="utf-8") - cmd = [binary, "--config", str(config_path), "--discover"] LOG.info("singer.discover binary=%s", binary) - try: - proc = subprocess.run( - cmd, - cwd=str(workdir.resolve()), - capture_output=True, - text=True, - timeout=timeout_seconds, - check=False, - ) - except subprocess.TimeoutExpired: - return None + with _private_tap_config(config, workdir) as config_path: + cmd = [binary, "--config", str(config_path), "--discover"] + try: + proc = subprocess.run( + cmd, + cwd=str(workdir.resolve()), + capture_output=True, + text=True, + timeout=timeout_seconds, + check=False, + ) + except subprocess.TimeoutExpired: + return None if proc.returncode != 0: # Tap doesn't support --discover (or failed) — caller decides. # We dump the full stderr to a sibling file so the user can diff --git a/fluid_build/cli/init.py b/fluid_build/cli/init.py index 2e8f74a2..efe1e96f 100644 --- a/fluid_build/cli/init.py +++ b/fluid_build/cli/init.py @@ -808,6 +808,10 @@ def _ensure_workspace(args, logger: logging.Logger) -> None: .fluid/copilot-memory.json .fluid/logs/ +# Generated per-run connector config (never commit; regenerated each run) +.fluid/debezium/ +.fluid/meltano/ + # Build outputs (never commit) runtime/ diff --git a/fluid_build/copilot/missions/checks.py b/fluid_build/copilot/missions/checks.py index 381ce148..5563bca9 100644 --- a/fluid_build/copilot/missions/checks.py +++ b/fluid_build/copilot/missions/checks.py @@ -41,6 +41,7 @@ import copy import logging +import re from dataclasses import dataclass, field from pathlib import Path from typing import Any, Callable, Dict, List, Optional, Tuple @@ -339,13 +340,38 @@ def _is_number(value: Any) -> bool: return isinstance(value, (int, float)) and not isinstance(value, bool) -def _compare(op: str, leaf: Any, expected: Any) -> Tuple[bool, str]: +def _display_leaf(concrete: str, leaf: Any) -> Any: + """The leaf as a diagnostic may quote it (``got ``). + + ``_redact_result`` cannot mask a quoted leaf afterwards: its pattern layer + reads ``password: expected == …, got 'x'`` as a ``password`` assignment + whose value is the word ``expected``, so ``'x'`` reached the terminal, + ``--json``, the scorecard file and the planner's LLM prompt. Mask it here + instead, by the path's key names, with the semantics ``redact_value`` + gives a mapping key: a leaf at or under a credential-named key is never + shown, and any other leaf goes through ``redact_value``. + """ + from fluid_build.observability.secret_redactor import ( + _REDACTED, + is_sensitive_key_name, + redact_value, + ) + + keys = [s for s in re.split(r"[.\[\]]", concrete) if s and s != "*" and not s.isdigit()] + if any(is_sensitive_key_name(key) for key in keys): + return _REDACTED + return redact_value(leaf) + + +def _compare(op: str, leaf: Any, expected: Any, *, shown: Any) -> Tuple[bool, str]: """Evaluate one leaf. Returns ``(ok, reason_if_not)``. Type mismatches - fail with a reason rather than raising — fail closed, never crash.""" + fail with a reason rather than raising — fail closed, never crash. + ``shown`` is what the reason quotes in place of the leaf + (:func:`_display_leaf`).""" if op == "eq": - return (leaf == expected, f"expected == {expected!r}, got {leaf!r}") + return (leaf == expected, f"expected == {expected!r}, got {shown!r}") if op == "ne": - return (leaf != expected, f"expected != {expected!r}, got {leaf!r}") + return (leaf != expected, f"expected != {expected!r}, got {shown!r}") if op == "contains": if isinstance(leaf, str): ok = isinstance(expected, str) and expected in leaf @@ -353,7 +379,7 @@ def _compare(op: str, leaf: Any, expected: Any) -> Tuple[bool, str]: ok = expected in leaf else: return (False, f"contains needs a string or array, got {type(leaf).__name__}") - return (ok, f"expected to contain {expected!r}, got {leaf!r}") + return (ok, f"expected to contain {expected!r}, got {shown!r}") # Ordered comparisons: both numeric, or both strings — never mixed. if _is_number(leaf) and _is_number(expected): @@ -371,7 +397,7 @@ def _compare(op: str, leaf: Any, expected: Any) -> Tuple[bool, str]: "gt": leaf > expected, "gte": leaf >= expected, }[op] - return (ok, f"expected {op} {expected!r}, got {leaf!r}") + return (ok, f"expected {op} {expected!r}, got {shown!r}") def _check_predicate( @@ -414,7 +440,9 @@ def _check_predicate( diagnostics.extend(f"{p}: path not found" for p in missing) failing = 0 for concrete, leaf in leaves: - ok, reason = _compare(criterion.op, leaf, criterion.value) + ok, reason = _compare( + criterion.op, leaf, criterion.value, shown=_display_leaf(concrete, leaf) + ) if not ok: failing += 1 diagnostics.append(f"{concrete}: {reason}") diff --git a/fluid_build/observability/secret_redactor.py b/fluid_build/observability/secret_redactor.py index 77fab5fa..fb536b1f 100644 --- a/fluid_build/observability/secret_redactor.py +++ b/fluid_build/observability/secret_redactor.py @@ -16,7 +16,7 @@ Two layers, in this order: -1. **Exact-value masking** (:func:`mask_known_secrets`) — every credential the +1. **Exact-value masking** (:func:`redact_known_values`) — every credential the process actually *holds* is registered here and removed from a log line by literal substring match. Delimiter-agnostic by construction: it cannot truncate, because it never has to guess where the value ends. @@ -129,7 +129,7 @@ # Fernet token — URL-safe base64 starting with the fixed ``gAAAAA`` header # emitted by ``cryptography.fernet`` (version byte 0x80 + timestamp). _FERNET_TOKEN_RE = re.compile(r"\bgAAAAA[A-Za-z0-9_-]{20,}") -# URL userinfo credentials: ``scheme://user:password@host`` (incl. the +# URL userinfo credentials: ``scheme://user:password@host`` (incl. the # pragma: allowlist secret # password-only ``scheme://:password@host`` form used by redis / AMQP / Celery # brokers — the username run is zero-or-more). The password is the run between # the first ``:`` after the scheme and the ``@`` that ends the authority. @@ -179,13 +179,13 @@ # password legally contains, so whichever set is chosen, some secret is masked # only up to the first one and its tail is emitted verbatim. A previous attempt # swapped ``;,}]`` for ``"&``: it fixed four inputs and broke others that this -# form handles correctly — ``jdbc:snowflake://h/?password="p@ss"&user=x`` and -# ```` went from fully masked to emitting the password +# form handles correctly — ``jdbc:snowflake://h/?password="p@ss"&user=x`` and # pragma: allowlist secret +# ```` went from fully masked to emitting the password # pragma: allowlist secret # verbatim, because a quoted branch that requires a specific character after # the closing quote falls through to an unquoted branch that can match ZERO # characters. Net: a strictly worse leak surface. # -# The correct fix for a value we HOLD is :func:`mask_known_secrets`, which runs +# The correct fix for a value we HOLD is :func:`redact_known_values`, which runs # first and matches the literal — no terminator to guess, no truncation # possible, and it covers every delimiter including whitespace (which this # pattern can never terminate on without destroying the surrounding log text). @@ -195,7 +195,7 @@ _ASSIGNMENT_RE = re.compile( r"(?ix)" r"(?P\b(?:[A-Za-z0-9_]{,128}_)?(?:" - r"api[_-]?key|authorization|aws_secret_access_key|secret[_-]access[_-]key|" + r"api[_-]?key|authorization|aws_secret_access_key|secret[_.-]access[_.-]key|secret[_.-]key|" r"client_secret|credentials?|oauth[_-]?token|password|passphrase|" r"private[_-]?key(?:_passphrase)?|session[_-]token|secret|token" r")\b)" @@ -229,7 +229,7 @@ # linear in the message length. _PRECEDING_SENSITIVE_KEY_RE = re.compile( r"(?ix)\b(?:[A-Za-z0-9_]*_)?(?:" - r"api[_-]?key|authorization|aws_secret_access_key|secret[_-]access[_-]key|" + r"api[_-]?key|authorization|aws_secret_access_key|secret[_.-]access[_.-]key|secret[_.-]key|" r"client_secret|credentials?|oauth[_-]?token|password|passphrase|" r"private[_-]?key(?:_passphrase)?|session[_-]token|secret|token" r")\b\s*[:=]\s*$" @@ -526,7 +526,7 @@ def known_secret_count() -> int: return len(_known_secrets) -def mask_known_secrets( +def redact_known_values( text: str, *, placeholder: str = _REDACTED, @@ -554,6 +554,17 @@ def mask_known_secrets( return text +# The pre-2026-10 name, kept so library callers keep working. Internal code +# calls ``redact_known_values``: CodeQL's sensitive-data heuristic treats a +# call to a function whose name contains "secret" as a SOURCE of secrets +# (``SensitiveFunctionCall``), so every call to the old name made the +# redactor's own OUTPUT look like a credential, and each log line or file +# that carried already-redacted text was reported as a clear-text leak. +# Names containing "redact" are on CodeQL's ``notSensitiveRegexp`` list +# (github/codeql shared/concepts/.../SensitiveDataHeuristics.qll). +mask_known_secrets = redact_known_values + + def _mask_assignment(match: "re.Match[str]") -> str: """Replace the value of a ``key=value`` assignment, keeping the key name. @@ -577,7 +588,7 @@ def redact_secret_text(text: str, *, extra_secrets: Iterable[str] = ()) -> str: # Layer 1 FIRST: remove the literals we hold before any pattern gets a # chance to chew a value into a truncated fragment. - redacted = mask_known_secrets(text, extra=extra_secrets) + redacted = redact_known_values(text, extra=extra_secrets) redacted = _BEARER_RE.sub(r"\1" + _REDACTED, redacted) # PEM blocks first: a multiline key block can itself contain ``.``/``=`` # runs that would otherwise be partially mangled by the token regexes. @@ -592,7 +603,7 @@ def redact_secret_text(text: str, *, extra_secrets: Iterable[str] = ()) -> str: for provider_re in _PROVIDER_KEY_RES: redacted = provider_re.sub(_REDACTED, redacted) redacted = _FERNET_TOKEN_RE.sub(_REDACTED, redacted) - # Mask the password in any ``scheme://user:password@host`` URL (scheme/user/ + # Mask the password in any ``scheme://user:password@host`` URL (scheme/user/ # pragma: allowlist secret # host preserved). Runs before the assignment regex so a credentialed URL is # scrubbed even when its key name isn't credential-shaped (``uri=https://u:p@h``, # ``connection_url=...``). diff --git a/fluid_build/providers/local/util/logging.py b/fluid_build/providers/local/util/logging.py index 07404ebd..395ed553 100644 --- a/fluid_build/providers/local/util/logging.py +++ b/fluid_build/providers/local/util/logging.py @@ -23,7 +23,7 @@ import re from typing import Any, Dict, List -from fluid_build.observability.secret_redactor import is_sensitive_key_name +from fluid_build.observability.secret_redactor import is_sensitive_key_name, redact_secret_text # Patterns for sensitive data that should be redacted SENSITIVE_PATTERNS = [ @@ -90,7 +90,11 @@ def redact_string(text: str) -> str: for pattern in SENSITIVE_PATTERNS: redacted = pattern.sub("[REDACTED]", redacted) - return redacted + # Then the shared redactor: the patterns above are a provider-local + # subset (no ``postgres://`` userinfo, no ``token=``, no credential this + # process registered), and ``_log_error`` passes a raw ``str(e)`` through + # here into the record's ``ctx``, which a JSON log formatter renders. + return redact_secret_text(redacted) def redact_dict(data: Dict[str, Any], max_depth: int = 10) -> Dict[str, Any]: diff --git a/fluid_build/providers/snowflake/util/logging.py b/fluid_build/providers/snowflake/util/logging.py index 22cef156..cee1b954 100644 --- a/fluid_build/providers/snowflake/util/logging.py +++ b/fluid_build/providers/snowflake/util/logging.py @@ -28,7 +28,7 @@ from fluid_build.observability.secret_redactor import ( _URL_USERINFO_RE, is_sensitive_key_name, - mask_known_secrets, + redact_known_values, ) # Patterns for sensitive data that should be redacted @@ -109,7 +109,7 @@ # deliberately UNCHANGED. No terminator set is leak-free — every # candidate character is one a real password may contain — so a # secret whose literal value is known must be masked by - # ``mask_known_secrets`` (run first, below), not by widening this. + # ``redact_known_values`` (run first, below), not by widening this. # An earlier attempt to widen it here regressed quoted values # followed by ``&`` or ``/`` into emitting the password verbatim. r"(?P[^\s;&,]{,256})" @@ -248,7 +248,7 @@ def redact_string(text: str) -> str: Runs the SAME two layers as the global redactor, in the same order: exact-value masking of the credentials this process holds - (``mask_known_secrets``, which shares one registry with the global layer so + (``redact_known_values``, which shares one registry with the global layer so the two cannot drift), then the pattern list below as the secondary net for values we do not hold. Only the placeholder differs — this layer's long-standing wire format is ``[REDACTED]``. @@ -265,7 +265,7 @@ def redact_string(text: str) -> str: # Layer 1 — literal match on known credentials. Delimiter-agnostic: a # password containing ``;`` ``,`` ``}`` ``]`` ``"`` ``&`` or a space is # masked whole, which no assignment pattern can guarantee. - redacted = mask_known_secrets(text, placeholder="[REDACTED]") + redacted = redact_known_values(text, placeholder="[REDACTED]") for pattern in SENSITIVE_PATTERNS: # Three substitution shapes, dispatched on the pattern's groups: diff --git a/tests/build_runners/test_catalog_auth_sink.py b/tests/build_runners/test_catalog_auth_sink.py index d7e2973e..57a6d1e0 100644 --- a/tests/build_runners/test_catalog_auth_sink.py +++ b/tests/build_runners/test_catalog_auth_sink.py @@ -278,10 +278,11 @@ def test_embedded_writes_env_references_and_the_server_inherits_the_environment( assert "debezium.sink.iceberg.scope=lakekeeper" in text assert _SECRET_VALUE not in text assert _SECRET_VALUE not in caplog.text - # No ``env=``: the server process inherits this one's environment, where - # Iceberg resolves env:LAKEKEEPER_CREDENTIAL. + # ``env=None``: the server process inherits this one's environment, where + # Iceberg resolves env:LAKEKEEPER_CREDENTIAL (the source has no password + # to add to it). assert len(fake_server.calls) == 1 - assert "env" not in fake_server.calls[0] + assert fake_server.calls[0].get("env") is None @pytest.mark.parametrize("value", [None, ""], ids=["unset", "empty"]) diff --git a/tests/integration/test_debezium_server_iceberg_live.py b/tests/integration/test_debezium_server_iceberg_live.py index 98f580c0..837e49a3 100644 --- a/tests/integration/test_debezium_server_iceberg_live.py +++ b/tests/integration/test_debezium_server_iceberg_live.py @@ -39,6 +39,7 @@ from fluid_build.build_runners.debezium.iceberg_sink import ( emit_debezium_iceberg_sink_config, ) +from fluid_build.build_runners.debezium.runner import render_application_properties from fluid_build.providers._iceberg_catalog import resolve_iceberg_catalog from tests._infrastructure.testcontainers_fixtures import _docker_available @@ -67,14 +68,16 @@ interval: 3s timeout: 3s retries: 30 + # Silo: the S3-compatible MinIO fork the Lakekeeper lane uses (minio/minio + # and minio/mc left Docker Hub on 2026-09-11). It ships ``mc`` too. minio: - image: minio/minio:latest + image: pgsty/silo:RELEASE.2026-09-03T13-18-01Z@sha256:b616a0cf8cb281e7e6bb3c9b1fb53875b4016a2878223925541c18f82d6c5ca3 environment: MINIO_ROOT_USER: minioadmin MINIO_ROOT_PASSWORD: minioadmin - command: server /data --console-address ":9001" + command: ["server", "/data"] mc: - image: minio/mc:latest + image: pgsty/silo:RELEASE.2026-09-03T13-18-01Z@sha256:b616a0cf8cb281e7e6bb3c9b1fb53875b4016a2878223925541c18f82d6c5ca3 depends_on: [minio] entrypoint: > /bin/sh -c " @@ -106,6 +109,7 @@ condition: service_healthy iceberg-rest: condition: service_started + env_file: [./debezium.env] volumes: - ./application.properties:/debezium/config/application.properties:ro """ @@ -150,10 +154,12 @@ """ -def _build_application_properties() -> str: +def _build_application_properties() -> tuple[str, dict[str, str]]: """The Iceberg sink block comes from forge's REAL deriver — the unit under - test — assembled with a standard Postgres source / JSON-format / file-offset - Debezium Server config.""" + test — and the file from the runner's REAL renderer, with a standard + Postgres source / JSON-format / file-offset Debezium Server config. The + renderer moves the credentials to the returned environment, which the + container gets through ``env_file``: the server must resolve them.""" binding = { "platform": "local", "format": "iceberg", @@ -175,33 +181,47 @@ def _build_application_properties() -> str: "s3.secret-access-key": "minioadmin", }, ) - # the runner's prefix loop (runner.py:468-469), applied verbatim - lines = ["quarkus.log.console.json=false", "debezium.sink.type=iceberg"] - lines += [f"debezium.sink.iceberg.{k}={v}" for k, v in derived.items()] - lines += [ + source = { + "offset.storage": "org.apache.kafka.connect.storage.FileOffsetBackingStore", + "offset.storage.file.filename": "/debezium/data/offsets.dat", + "offset.flush.interval.ms": "0", + "connector.class": "io.debezium.connector.postgresql.PostgresConnector", + "database.hostname": "postgres", + "database.port": "5432", + "database.user": "postgres", + "database.password": "postgres", # pragma: allowlist secret + "database.dbname": "appdb", + "database.server.id": "1234", + "topic.prefix": "cdcsrv", + "plugin.name": "pgoutput", + "slot.name": "dbz_slot", + "publication.name": "dbz_pub", + "publication.autocreate.mode": "filtered", + "snapshot.mode": "initial", + "table.include.list": "public.orders", + } + props, env = render_application_properties(source, "iceberg", derived) + lines = [ + props, "debezium.format.value=json", "debezium.format.value.schemas.enable=true", "debezium.format.key=json", "debezium.format.key.schemas.enable=true", - "debezium.source.offset.storage=org.apache.kafka.connect.storage.FileOffsetBackingStore", - "debezium.source.offset.storage.file.filename=/debezium/data/offsets.dat", - "debezium.source.offset.flush.interval.ms=0", - "debezium.source.connector.class=io.debezium.connector.postgresql.PostgresConnector", - "debezium.source.database.hostname=postgres", - "debezium.source.database.port=5432", - "debezium.source.database.user=postgres", - "debezium.source.database.password=postgres", - "debezium.source.database.dbname=appdb", - "debezium.source.database.server.id=1234", - "debezium.source.topic.prefix=cdcsrv", - "debezium.source.plugin.name=pgoutput", - "debezium.source.slot.name=dbz_slot", - "debezium.source.publication.name=dbz_pub", - "debezium.source.publication.autocreate.mode=filtered", - "debezium.source.snapshot.mode=initial", - "debezium.source.table.include.list=public.orders", ] - return "\n".join(lines) + "\n" + return "\n".join(lines) + "\n", env + + +def _assert_credentials_only_in_env(props: str, env: dict[str, str]) -> None: + """No credential in the file: the server resolves each from its environment.""" + assert "debezium.source.database.password=${FLUID_DBZ_SOURCE_DATABASE_PASSWORD}" in props + assert ( + "debezium.sink.iceberg.s3.secret-access-key=" + "${FLUID_DBZ_SINK_ICEBERG_S3_SECRET_ACCESS_KEY}" in props + ) + assert env == { + "FLUID_DBZ_SOURCE_DATABASE_PASSWORD": "postgres", # pragma: allowlist secret + "FLUID_DBZ_SINK_ICEBERG_S3_SECRET_ACCESS_KEY": "minioadmin", # pragma: allowlist secret + } def _compose(args, cwd, project): @@ -219,11 +239,13 @@ def test_debezium_server_iceberg_lands_rows(tmp_path: Path): (tmp_path / "docker-compose.yml").write_text(_COMPOSE) (tmp_path / "seed.sql").write_text(_SEED) (tmp_path / "read_back.py").write_text(_READ_BACK) - props = _build_application_properties() + props, env = _build_application_properties() (tmp_path / "application.properties").write_text(props) + (tmp_path / "debezium.env").write_text("".join(f"{k}={v}\n" for k, v in env.items())) # the deriver's output is asserted directly (cheap, runs even on slow CI) assert "debezium.sink.iceberg.type=rest" in props + _assert_credentials_only_in_env(props, env) assert "debezium.sink.iceberg.io-impl=org.apache.iceberg.aws.s3.S3FileIO" in props assert "debezium.sink.iceberg.table-namespace=cdc" in props assert "iceberg.control.topic" not in props # KC-only key must not leak diff --git a/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py b/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py new file mode 100644 index 00000000..93fb15b6 --- /dev/null +++ b/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py @@ -0,0 +1,418 @@ +# 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. + +"""Credentials stay out of generated files, run records, log context and +mission diagnostics. + +One test group per CodeQL alert the 2026-10 triage confirmed on main: + +* #241 — ``fluid mission`` quoted a contract's inline credential in a failing + predicate's diagnostic (terminal, ``--json``, scorecard, LLM prompt). +* #231 — the shared redactor missed ``aws.secret.access.key=`` and the other + dotted Java/Hadoop spellings, so a Kafka Connect trace kept an S3 key in the + run record. +* #292 — the local provider's log context carried a raw ``postgres://`` URL + with its password. +* #211 — Debezium Server's ``application.properties`` held the resolved + source password, 0o644 before a chmod, under the contract directory. +* #53 / #54 — the Singer ``tap_config.json`` held the resolved password, + 0o644, never deleted, under the contract directory. + +Plus the rename that stops CodeQL reading the redactor's own output as a +secret (``mask_known_secrets`` → ``redact_known_values``). +""" + +from __future__ import annotations + +import ast +import json +import logging +import shutil +import stat +import subprocess +import sys +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from fluid_build.observability import secret_redactor as sr + +_SECRET = "Hunter2-Sup3rS3cret-0xBEEF" # pragma: allowlist secret +_S3_KEY = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" # pragma: allowlist secret +_NEWLINE_VALUE = "a\nb" # pragma: allowlist secret +_SECRET_REF = "env://PG_PASSWORD_T211" # pragma: allowlist secret + + +# ── rename: the redactor's output is not a CodeQL "secret" source ───────── + + +def test_old_name_still_imports_for_library_callers(): + assert sr.mask_known_secrets is sr.redact_known_values + + +def test_no_module_calls_the_old_name(): + """CodeQL treats a call to a function named like ``*secret*`` as a source + of secrets. A call to ``mask_known_secrets`` made the redactor's OUTPUT a + "secret", and 14 alerts followed it into log lines and files that only + ever held redacted text. Call ``redact_known_values``.""" + root = Path(sr.__file__).resolve().parents[1] + callers = [] + for path in root.rglob("*.py"): + tree = ast.parse(path.read_text(encoding="utf-8")) + for node in ast.walk(tree): + if isinstance(node, ast.Call): + func = node.func + name = func.attr if isinstance(func, ast.Attribute) else getattr(func, "id", "") + if name == "mask_known_secrets": + callers.append(f"{path.relative_to(root)}:{node.lineno}") + assert callers == [] + + +# ── #231: dotted Java/Hadoop secret key spellings ───────────────────────── + + +@pytest.mark.parametrize( + "key", + [ + "aws.secret.access.key", + "secret.access.key", + "fs.s3a.secret.key", + "secret_key", + "s3.secret-access-key", + ], +) +def test_dotted_secret_key_assignment_is_masked(key): + out = sr.redact_secret_text(f"config {{{key}={_S3_KEY}, topic=orders}}") + assert _S3_KEY not in out + assert key in out + assert "topic=orders" in out + + +def test_prose_about_a_secret_key_is_left_alone(): + text = "rotate the secret key monthly" + assert sr.redact_secret_text(text) == text + + +def test_dotted_key_before_a_log_placeholder_is_masked(): + record = logging.LogRecord( + "t", logging.ERROR, __file__, 1, "aws.secret.access.key=%s", (_S3_KEY,), None + ) + sr.SecretRedactingFilter().filter(record) + assert _S3_KEY not in record.getMessage() + + +def test_run_record_keeps_no_s3_key_from_a_connect_trace(tmp_path): + from fluid_build.build_runners._state import FileStateStore + + record = { + "run_id": "run_1", + "state": "failed", + "facets": { + "failed_task_traces": [ + "org.apache.kafka.connect.errors.ConnectException: " + f"{{aws.secret.access.key={_S3_KEY}, s3.bucket.name=lake}}" + ] + }, + } + FileStateStore(tmp_path).write_run_record("prod", "build", record) + written = (tmp_path / "runs/prod/build/runs/run_1.json").read_text() + assert _S3_KEY not in written + assert "s3.bucket.name=lake" in written + + +# ── #241: mission diagnostics never quote a credential leaf ─────────────── + + +def _mission(tmp_path, criteria): + from fluid_build.copilot.missions.checks import run_mission_checks + from fluid_build.copilot.missions.spec import CriterionSpec, MissionSpec + + contract = tmp_path / "contract.fluid.yaml" + contract.write_text( + "fluidVersion: '0.7.1'\n" + "kind: DataProduct\n" + "id: demo.p\n" + "name: p\n" + "exposes:\n" + " - exposeId: e1\n" + " binding:\n" + " platform: postgres\n" + " location:\n" + " database: sales\n" + f" password: '{_SECRET}'\n" + f" api_token: '{_SECRET}'\n" + f" settings: {{user: app, password: '{_SECRET}'}}\n", + encoding="utf-8", + ) + spec = MissionSpec( + name="no-inline-creds", + description="d", + goal="use env refs", + success_criteria=tuple( + CriterionSpec(check="predicate", path=p, op=op, value=v, value_provided=True) + for p, op, v in criteria + ), + ) + return spec, run_mission_checks(spec, contract) + + +def test_failing_predicate_on_a_credential_does_not_quote_it(tmp_path, capsys): + from fluid_build.cli.mission import _render_scorecard + from fluid_build.copilot.missions.planner import build_plan_prompt + + spec, scorecard = _mission( + tmp_path, + [ + ("exposes[*].binding.location.password", "eq", "${PG_PASSWORD}"), + ("exposes[*].binding.location.api_token", "contains", "{{"), + ], + ) + _render_scorecard(spec, scorecard) + printed = capsys.readouterr().out + + assert "***REDACTED***" in printed + assert _SECRET not in printed + assert _SECRET not in json.dumps(scorecard.to_dict()) + assert _SECRET not in build_plan_prompt(spec, scorecard) + + +def test_a_mapping_leaf_keeps_its_ordinary_keys_readable(tmp_path): + _spec, scorecard = _mission( + tmp_path, [("exposes[*].binding.location.settings", "eq", {"user": "app"})] + ) + diagnostics = " ".join(line for r in scorecard.results for line in r.diagnostics) + assert _SECRET not in diagnostics + assert "'user': 'app'" in diagnostics + + +def test_an_ordinary_leaf_is_still_quoted(tmp_path): + _spec, scorecard = _mission( + tmp_path, [("exposes[*].binding.location.database", "eq", "orders")] + ) + diagnostics = " ".join(line for r in scorecard.results for line in r.diagnostics) + assert "got 'sales'" in diagnostics + + +# ── #292: the local provider's log context ─────────────────────────────── + + +def test_local_sql_error_context_masks_a_postgres_url_password(tmp_path): + from fluid_build.providers.local.local import LocalProvider + + records: list[logging.LogRecord] = [] + + class _Capture(logging.Handler): + def emit(self, record): + records.append(record) + + log = logging.getLogger("test.local.ctx") + log.setLevel(logging.DEBUG) + log.propagate = False + log.addHandler(_Capture()) + url = f"postgres://etl:{_SECRET}@nohost/x.csv" + LocalProvider(logger=log, anchor_dir=str(tmp_path))._log_error( + "local_sql_error", {"error": f'Cannot access file "{url}"', "i": 0} + ) + assert records, "nothing was logged" + assert _SECRET not in repr(records[-1].__dict__) + assert "nohost/x.csv" in repr(records[-1].__dict__) + + +def test_local_redact_sql_masks_a_postgres_url_password(): + from fluid_build.providers.local.util.logging import redact_sql + + out = redact_sql(f"SELECT * FROM read_csv('postgres://etl:{_SECRET}@nohost/x.csv')") + assert _SECRET not in out + + +# ── #211: Debezium Server gets its credentials from the environment ────── + + +def test_credential_properties_render_as_env_expressions(): + from fluid_build.build_runners.debezium.runner import render_application_properties + + text, env = render_application_properties( + {"database.user": "app", "database.password": _SECRET, "slot.name": "s"}, + "iceberg", + { + "s3.secret-access-key": _S3_KEY, + "credential": "env:LAKEKEEPER_CREDENTIAL", + "rest.auth.type": "oauth2", + "jdbc.password": "", + }, + ) + assert _SECRET not in text and _S3_KEY not in text + assert "debezium.source.database.password=${FLUID_DBZ_SOURCE_DATABASE_PASSWORD}" in text + assert ( + "debezium.sink.iceberg.s3.secret-access-key=" + "${FLUID_DBZ_SINK_ICEBERG_S3_SECRET_ACCESS_KEY}" in text + ) + # Iceberg's own env reference, a non-secret auth key and an empty value + # are written as they are. + assert "debezium.sink.iceberg.credential=env:LAKEKEEPER_CREDENTIAL" in text + assert "debezium.sink.iceberg.rest.auth.type=oauth2" in text + assert "debezium.sink.iceberg.jdbc.password=\n" in text + "\n" + assert "debezium.source.database.user=app" in text + assert env == { + "FLUID_DBZ_SOURCE_DATABASE_PASSWORD": _SECRET, + "FLUID_DBZ_SINK_ICEBERG_S3_SECRET_ACCESS_KEY": _S3_KEY, + } + + +def test_a_control_character_in_a_credential_still_fails_closed(): + from fluid_build.build_runners.debezium.runner import render_application_properties + + with pytest.raises(ValueError, match="control character"): + render_application_properties({"database.password": _NEWLINE_VALUE}, "kafka", {}) + + +def test_two_properties_on_one_env_name_fail_closed(): + from fluid_build.build_runners.debezium.runner import render_application_properties + + with pytest.raises(ValueError, match="FLUID_DBZ_SINK_KAFKA_A_PASSWORD"): + render_application_properties({}, "kafka", {"a.password": "x", "a-password": "y"}) + + +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX file modes and a shell script") +def test_embedded_server_file_holds_no_password_and_server_gets_it(tmp_path, monkeypatch): + from fluid_build.build_runners.debezium import runner as dbz + + monkeypatch.setenv("PG_PASSWORD_T211", _SECRET) + captured = tmp_path / "server-env.txt" + server = tmp_path / "debezium-server" + server.write_text( + f"#!/bin/sh\nprintf '%s' \"$FLUID_DBZ_SOURCE_DATABASE_PASSWORD\" > '{captured}'\nexit 3\n" + ) + server.chmod(0o700) + ctx = SimpleNamespace( + workdir=str(tmp_path), + product_id="sales.orders", + build_id="b1", + run_id="r1", + source=SimpleNamespace( + kind="postgres", + connection=SimpleNamespace(raw={"host": "db", "user": "app", "secretRef": _SECRET_REF}), + streams=["public.orders"], + ), + contract={}, + sink=None, + env={}, + ) + dbz._execute_debezium_server( + ctx, + {"engine": "debezium", "properties": {}}, + {"mode": "embedded"}, + {"server": {"sink": {"type": "kafka"}}, "server_binary": str(server)}, + "initial", + "2026-10-13T00:00:00Z", + 0.0, + ) + config = tmp_path / ".fluid/debezium/sales.orders/b1/application.properties" + assert _SECRET not in config.read_text() + assert stat.S_IMODE(config.stat().st_mode) == 0o600 + assert captured.read_text() == _SECRET + + +# ── #53 / #54: the Singer tap config is private and short-lived ────────── + + +def _fake_tap(tmp_path: Path, exit_code: int, sleep: float = 0.0) -> Path: + """A tap that records its ``--config`` file's mode and content.""" + tap = tmp_path / "tap-fake" + seen = tmp_path / "tap-saw.json" + tap.write_text( + f"#!{sys.executable}\n" + "import json, os, stat, sys, time\n" + "path = sys.argv[sys.argv.index('--config') + 1]\n" + "with open(path) as fh: body = fh.read()\n" + "mode = stat.S_IMODE(os.stat(path).st_mode)\n" + "parent = stat.S_IMODE(os.stat(os.path.dirname(path)).st_mode)\n" + f"json.dump({{'path': path, 'mode': mode, 'parent': parent, 'body': body}}," + f" open({str(seen)!r}, 'w'))\n" + f"time.sleep({sleep})\n" + f"sys.exit({exit_code})\n" + ) + tap.chmod(0o700) + return tap + + +def _tap_config(): + return {"host": "db", "user": "alice", "password": _SECRET} + + +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX file modes and a script tap") +def test_invoke_tap_config_is_private_and_removed(tmp_path): + from fluid_build.build_runners.meltano.runner import invoke_tap + + workdir = tmp_path / ".fluid/meltano/p/b" + workdir.mkdir(parents=True) + stale = workdir / "tap_config.json" + stale.write_text(json.dumps(_tap_config())) # an earlier release's copy + + invoke_tap(str(_fake_tap(tmp_path, 0)), config=_tap_config(), workdir=workdir) + + seen = json.loads((tmp_path / "tap-saw.json").read_text()) + assert json.loads(seen["body"])["password"] == _SECRET + assert seen["mode"] == 0o600 and seen["parent"] == 0o700 + assert not Path(seen["path"]).exists() + assert not Path(seen["path"]).is_relative_to(tmp_path) + assert not stale.exists() + assert all(_SECRET not in p.read_text() for p in workdir.rglob("*") if p.is_file()) + + +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX file modes and a script tap") +def test_discover_removes_the_config_after_a_failed_tap(tmp_path): + from fluid_build.build_runners.meltano.runner import discover_tap_catalog + + workdir = tmp_path / "w" + assert ( + discover_tap_catalog(str(_fake_tap(tmp_path, 1)), config=_tap_config(), workdir=workdir) + is None + ) + seen = json.loads((tmp_path / "tap-saw.json").read_text()) + assert not Path(seen["path"]).exists() + assert all(_SECRET not in p.read_text() for p in workdir.rglob("*") if p.is_file()) + + +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX file modes and a script tap") +def test_discover_removes_the_config_after_a_timeout(tmp_path): + from fluid_build.build_runners.meltano.runner import discover_tap_catalog + + tap = _fake_tap(tmp_path, 0, sleep=30) + assert ( + discover_tap_catalog( + str(tap), config=_tap_config(), workdir=tmp_path / "w", timeout_seconds=2 + ) + is None + ) + seen = json.loads((tmp_path / "tap-saw.json").read_text()) + assert not Path(seen["path"]).exists() + + +# ── fluid init: generated connector config is git-ignored ──────────────── + + +@pytest.mark.skipif(shutil.which("git") is None, reason="needs git") +def test_init_gitignore_block_ignores_generated_connector_config(tmp_path): + from fluid_build.cli.init import FLUID_GITIGNORE_BLOCK + + subprocess.run(["git", "init", "-q", str(tmp_path)], check=True) + (tmp_path / ".gitignore").write_text(FLUID_GITIGNORE_BLOCK) + for rel in ( + ".fluid/debezium/sales.orders/b1/application.properties", + ".fluid/meltano/sales.orders/b1/tap_state.json", + ): + result = subprocess.run(["git", "-C", str(tmp_path), "check-ignore", "-q", rel]) + assert result.returncode == 0, rel diff --git a/tests/security/test_security_regressions_round3.py b/tests/security/test_security_regressions_round3.py index 12529205..276cc207 100644 --- a/tests/security/test_security_regressions_round3.py +++ b/tests/security/test_security_regressions_round3.py @@ -329,7 +329,7 @@ def health_check(self): # pragma: no cover - not exercised # ``jdbc:snowflake://h/?password="p@ss"&user=x`` from fully masked into # emitting the password verbatim. Net: strictly worse. # -# The fix is the exact-value layer (``mask_known_secrets``, borrowed from +# The fix is the exact-value layer (``redact_known_values``, borrowed from # ``misprint``): at redaction time the credential's literal value is already # held, so it is matched literally — delimiter-agnostic, cannot truncate. The # pattern layer is left EXACTLY as shipped and is only the net for values we From 05a1e39bca9d135159a5f26afa1da94da7fccf74 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:59:58 +0200 Subject: [PATCH 2/6] fix(debezium): an unwritable application.properties is a failed run, not a crash _atomic_write_bytes raises FileExistsError on a stale sibling temp file; return a failed RunResult naming the path. The live test's read-back pins pyiceberg 0.11.1, the first release that parses the format-version 3 metadata the debezium-server-iceberg image now writes. --- fluid_build/build_runners/debezium/runner.py | 5 ++- .../test_debezium_server_iceberg_live.py | 3 +- ...tials_stay_out_of_files_and_diagnostics.py | 33 +++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/fluid_build/build_runners/debezium/runner.py b/fluid_build/build_runners/debezium/runner.py index 4c69e8c0..aa261d74 100644 --- a/fluid_build/build_runners/debezium/runner.py +++ b/fluid_build/build_runners/debezium/runner.py @@ -644,7 +644,10 @@ def _execute_debezium_server( # world-readable, and no chmod whose failure could be ignored. A # hand-written sink key the credential rule does not name still lands # in this file. - _atomic_write_bytes(config_path, properties.encode("utf-8"), mode=0o600) + try: + _atomic_write_bytes(config_path, properties.encode("utf-8"), mode=0o600) + except OSError as exc: + return _failed(ctx, started_at, t_start, f"could not write {config_path}: {exc}") # Validate any contract-supplied ``server_binary`` before it becomes # ``argv[0]`` (arbitrary-binary-execution guard). A value with a path diff --git a/tests/integration/test_debezium_server_iceberg_live.py b/tests/integration/test_debezium_server_iceberg_live.py index 837e49a3..fd9ef5ec 100644 --- a/tests/integration/test_debezium_server_iceberg_live.py +++ b/tests/integration/test_debezium_server_iceberg_live.py @@ -297,7 +297,8 @@ def test_debezium_server_iceberg_lands_rows(tmp_path: Path): "python:3.11-slim", "sh", "-c", - "pip -q install 'pyiceberg[s3fs]==0.8.1' pyarrow >/dev/null 2>&1 " + # 0.11.1 parses format-version 3 metadata, which the server writes + "pip -q install 'pyiceberg[s3fs]==0.11.1' pyarrow >/dev/null 2>&1 " "&& python /read_back.py", ], capture_output=True, diff --git a/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py b/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py index 93fb15b6..c0f1607e 100644 --- a/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py +++ b/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py @@ -325,6 +325,39 @@ def test_embedded_server_file_holds_no_password_and_server_gets_it(tmp_path, mon assert captured.read_text() == _SECRET +def test_an_unwritable_config_is_a_failed_run_not_a_crash(tmp_path, monkeypatch): + from fluid_build.build_runners.debezium import runner as dbz + from fluid_build.credentials import encrypted_store + + def refuse(*_a, **_k): + raise FileExistsError(17, "File exists") + + monkeypatch.setattr(encrypted_store, "_atomic_write_bytes", refuse) + ctx = SimpleNamespace( + workdir=str(tmp_path), + product_id="p", + build_id="b", + run_id="r", + source=SimpleNamespace( + kind="postgres", connection=SimpleNamespace(raw={"host": "db"}), streams=[] + ), + contract={}, + sink=None, + env={}, + ) + result = dbz._execute_debezium_server( + ctx, + {"engine": "debezium", "properties": {}}, + {"mode": "embedded"}, + {"server": {"sink": {"type": "kafka"}}}, + "initial", + "2026-10-13T00:00:00Z", + 0.0, + ) + assert result.state.value == "failed" + assert "could not write" in (result.error or "") + + # ── #53 / #54: the Singer tap config is private and short-lived ────────── From d1b791064a20b70499daaea9e4490886c72b5103 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Tue, 13 Oct 2026 22:32:51 +0200 Subject: [PATCH 3/6] fix(security): close the gaps the defect scan found in the credential fixes - fluid mission: mask a leaf only under a key that NAMES a credential (new secret_redactor.names_a_credential: camelCase/kebab read as snake, must end in a credential word, one vocabulary with the assignment rule). maxTokensPerDay, authz, secretRef, tokenEnv and author show again; privateKey, apiKey, clientSecret are masked. A self-referential leaf no longer flips a predicate (RecursionError -> masked). - local provider: run the Snowflake twin's first two layers (held credentials whole, URL userinfo) BEFORE the local patterns, instead of the full shared redactor after them: a held secret with ; or @ is no longer cut short, and dotted table names are not JWT-masked. - Snowflake twin: same secret.access.key / secret.key spellings (extend both). - Debezium: write application.properties in place at 0600 (O_NOFOLLOW, fchmod, truncates a 0644 copy from an earlier release) instead of a pid-named temp file a killed run could leave behind; env: stays literal for sink keys only; a credential's control-character error names the key, not the value; the credential rule also covers authorization, authParams, connectionstring and API/private/account/keystore/SSE keys. - Meltano: target_config.json gets the same private temp dir as the tap. - fluid init: **/.fluid/debezium/ and **/.fluid/meltano/ match in every product directory, not only the workspace root. - The inline-secret warning says the value is passed to Debezium Server. - CHANGELOG claims scoped to what changed. --- .secrets.baseline | 6 +- CHANGELOG.md | 74 ++++--- fluid_build/build_runners/debezium/runner.py | 65 ++++-- .../kafka_connect/iceberg_sink_validation.py | 5 +- fluid_build/build_runners/meltano/runner.py | 61 +++--- fluid_build/cli/init.py | 6 +- fluid_build/copilot/missions/checks.py | 16 +- fluid_build/observability/secret_redactor.py | 41 +++- fluid_build/providers/local/util/logging.py | 20 +- .../providers/snowflake/util/logging.py | 2 +- .../test_catalog_auth_validation.py | 2 +- ...tials_stay_out_of_files_and_diagnostics.py | 202 ++++++++++++++++-- 12 files changed, 372 insertions(+), 128 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index e5013d79..821f75f0 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -427,7 +427,7 @@ "filename": "fluid_build/observability/secret_redactor.py", "hashed_secret": "1348b145fa1a555461c1b790a2f66614781091e9", "is_verified": false, - "line_number": 156 + "line_number": 165 } ], "fluid_build/providers/_sql_safety.py": [ @@ -470,7 +470,7 @@ "filename": "fluid_build/providers/local/util/logging.py", "hashed_secret": "9d4e1e23bd5b727046a9e3b4b7db57bd8d6ee684", "is_verified": false, - "line_number": 45 + "line_number": 49 } ], "fluid_build/providers/snowflake/types.py": [ @@ -2154,5 +2154,5 @@ } ] }, - "generated_at": "2026-10-13T19:51:44Z" + "generated_at": "2026-10-13T20:32:20Z" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 4094f777..7f6fe089 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -113,22 +113,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 with `env:` or carries a `${...}` expression does not warn. Under `fluid validate --strict` the warning fails validation; use `sink.catalogAuth`, or an `env:` value. -- The embedded Debezium Server's `application.properties` no longer holds credentials. A - property whose name carries `password`, `secret`, `token`, `credential`, `jaas.config` - or `connection.string` is written as `${FLUID_DBZ_}` (for example +- The embedded Debezium Server's `application.properties` no longer holds the credentials + forge-cli resolves or derives. A property whose name carries `password`, `secret`, + `token`, `credential`, `authorization`, `authParams`, `jaas.config`, a connection + string, or an API, private, account, keystore or SSE key is written as + `${FLUID_DBZ_}` (for example `debezium.source.database.password=${FLUID_DBZ_SOURCE_DATABASE_PASSWORD}`), and forge-cli passes the value to the server in that environment variable. A `server_binary` wrapper - that replaces the environment must pass the `FLUID_DBZ_*` variables through. An - `env:` value is still written as it is. -- A Meltano / Singer tap now reads its `--config` file from a private temporary - directory that is removed when the tap exits, instead of - `.fluid/meltano///tap_config.json`. A copy an earlier release left there - is deleted on the next run. A tap that writes a refreshed token back to its config file - loses that write; the next run resolves the credential again. -- `fluid init` now git-ignores `.fluid/debezium/` and `.fluid/meltano/`. A project whose - `.gitignore` already has the fluid-cli block does not get the new lines; add them - yourself, and check that no `application.properties` or `tap_config.json` under those - directories was committed with a credential in it. + that replaces the environment must pass the `FLUID_DBZ_*` variables through. A sink + `env:` value is still written as it is. A hand-written sink key outside that + list is written as before, in a file that is now created at mode 0600. +- A Meltano / Singer tap and target now read their `--config` file from a private + temporary directory that is removed when the tap or target exits, instead of + `.fluid/meltano///tap_config.json` and `target_config.json`. A copy an + earlier release left there is deleted on the next run. A tap that writes a refreshed + token back to its config file loses that write; the next run resolves the credential + again. +- `fluid init` now git-ignores `**/.fluid/debezium/` and `**/.fluid/meltano/`, in every + product directory of the workspace. A project whose `.gitignore` already has the + fluid-cli block does not get the new lines; add them yourself, and check that no + `application.properties`, `tap_config.json` or `target_config.json` under those + directories was committed with a credential in it. Rotate any you find. ### Added @@ -431,25 +436,32 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 state rm` remediation. ### Security -- **Credentials no longer reach disk in generated connector config.** The embedded Debezium - Server's `application.properties` held the source database password that a `secretRef` - resolved, under the contract directory, briefly at mode 0644 and kept after the run; the - Singer `tap_config.json` held it at 0644, was never deleted, and neither file was - git-ignored, so `git add -A` staged the password. The Debezium credential now goes to the - server's environment and the file is created at 0600; the tap config lives 0600 in a - private temporary directory for the life of the tap. (CodeQL alerts 211, 53, 54.) -- **`fluid mission` no longer quotes a contract's inline credential.** A failing predicate on - a path such as `exposes[*].binding.location.password` printed `got ''` to the - terminal, `--json`, the stored scorecard and the planner's LLM prompt, because the - redactor read `password: expected == …` as the assignment. A leaf at or under a - credential-named key now shows as `***REDACTED***`. (CodeQL alert 241.) +- **The Debezium and Singer config files forge-cli generates no longer keep the source or + destination password on disk.** The embedded Debezium Server's `application.properties` + held the source database password a `secretRef` resolved, under the contract directory, + briefly at mode 0644 and kept after the run. The Singer `tap_config.json` and + `target_config.json` held the source and warehouse passwords at 0644 and were never + deleted. None of them was git-ignored, so `git add -A` staged the password. The Debezium + credentials now go to the server's environment and the file is created at 0600; the + Singer configs live 0600 in a private temporary directory for the life of the tap or + target. (CodeQL alerts 211, 53, 54.) +- **A failing `fluid mission` predicate no longer quotes a contract's inline credential.** A + predicate on a path such as `exposes[*].binding.location.password` printed + `got ''` to the terminal, `--json`, the stored scorecard and the planner's LLM + prompt, because the redactor read `password: expected == …` as the assignment. A leaf at + or under a key that names a credential (`password`, `apiKey`, `privateKey`, + `clientSecret`, `sessionToken`, …) now shows as `***REDACTED***`; a key that only + contains such a word (`maxTokensPerDay`, `secretRef`, `tokenEnv`) still shows its value. + (CodeQL alert 241.) - **The shared redactor masks `aws.secret.access.key=`, `secret.access.key=` and `fs.s3a.secret.key=`.** It knew only the `_` and `-` spellings, so a Kafka Connect task - trace stored in a run record kept an S3 secret key. (CodeQL alert 231.) -- **The local provider's log context is redacted by the shared redactor.** Its own - patterns missed `postgres://user:password@` URLs and `token=`, and a failed SQL action - put the raw error into the log record's context, which a JSON log formatter renders. - (CodeQL alert 292.) + trace stored in a run record kept an S3 secret key. The Snowflake provider's redactor + masks the same spellings. (CodeQL alert 231.) +- **The local provider's log context masks the credentials forge-cli holds and URL + userinfo.** A failed SQL action put the raw error into the log record's context, which a + JSON log formatter renders, and the provider's own patterns knew only `postgresql://`. + It now runs the same two layers first that the Snowflake provider does: each registered + credential masked whole, then any `scheme://user:password@`. (CodeQL alert 292.) - `mask_known_secrets` is now `redact_known_values`; the old name stays as an alias. CodeQL treats a call to a function named like `*secret*` as a source of secrets, so every caller of the redactor was reported as logging or storing a credential it had already masked. diff --git a/fluid_build/build_runners/debezium/runner.py b/fluid_build/build_runners/debezium/runner.py index aa261d74..451a9882 100644 --- a/fluid_build/build_runners/debezium/runner.py +++ b/fluid_build/build_runners/debezium/runner.py @@ -491,15 +491,31 @@ def _escape_properties_value(value: str) -> str: return escaped +def _write_private_text(path: Path, text: str) -> None: + """Write ``text`` to ``path``, readable by this user only (0o600).""" + flags = os.O_WRONLY | os.O_CREAT | os.O_TRUNC | getattr(os, "O_NOFOLLOW", 0) + fd = os.open(path, flags, 0o600) + with os.fdopen(fd, "w", encoding="utf-8") as fh: + if hasattr(os, "fchmod"): + os.fchmod(fh.fileno(), 0o600) + fh.write(text) + + #: A property whose VALUE is a credential: Debezium's PASSWORD-typed fields #: (``database.password``, ``*.ssl.*.password``), the Iceberg / S3 / JDBC / #: GCS / ADLS secrets (``credential``, ``token``, ``s3.secret-access-key``, -#: ``s3.session-token``, ``jdbc.password``, ``adls.sas-token.*``), a Kafka JAAS -#: blob, and a Mongo connection string (it may carry userinfo). Narrower than +#: ``s3.session-token``, ``jdbc.password``, ``adls.sas-token.*``, +#: ``adls.auth.shared-key.account.key``, ``s3.sse.key``), an HTTP +#: ``header.Authorization``, a Kafka JAAS blob or PEM ``ssl.keystore.key``, a +#: Pulsar ``authParams``, and a Mongo or Event Hubs connection string (it may +#: carry userinfo or a shared access key). Narrower than #: ``is_sensitive_key_name`` on purpose: ``rest.auth.type`` and -#: ``oauth2-server-uri`` are not secrets, and stay readable in the file. +#: ``oauth2-server-uri`` are not secrets, and stay readable in the file. A +#: false positive only moves a value to the environment. _CREDENTIAL_PROPERTY_RE = re.compile( - r"(?i)password|secret|token|credential|jaas\.config|connection\.string" + r"(?i)password|secret|token|credential|authorization|auth[._-]?params|jaas\.config" + r"|connection[._-]?string|api[._-]?key|private[._-]?key|account[._-]?key" + r"|keystore\.key|sse\.key" ) @@ -520,8 +536,10 @@ def render_application_properties( (DBZ-2886). So no credential is written to disk: the file sits under the contract directory, where ``git add -A`` would stage it. - An ``env:`` value is written as it is: it is Iceberg's own reference - to an environment variable (``sink.catalogAuth``), not a secret. + A sink ``env:`` value is written as it is: it is Iceberg's own + reference to an environment variable (``sink.catalogAuth``), not a secret. + A source property gets no such reading, so a source password that starts + with ``env:`` still goes to the environment. Raises ``ValueError`` for a control character in a key or value (:func:`_properties_line`), or for two properties whose names map to the @@ -529,11 +547,21 @@ def render_application_properties( """ env: Dict[str, str] = {} - def line(key: str, value: object) -> str: - rendered = _properties_line(key, value) + def line(key: str, value: object, *, env_refs: bool = False) -> str: text = str(value) - if not text or text.startswith("env:") or not _CREDENTIAL_PROPERTY_RE.search(key): - return rendered + if ( + not text + or (env_refs and text.startswith("env:")) + or not _CREDENTIAL_PROPERTY_RE.search(key) + ): + return _properties_line(key, value) + # The value never reaches the file, so only a control character in + # the KEY could inject a line; the value is still refused (no + # credential has one, a trailing newline from a secret file is a + # mistake), but the error names the key, never the value. + _properties_line(key, "") + if any(ch in text for ch in "\r\n\f\x00"): + raise ValueError(f"debezium config entry {key} has a control character in its value") name = "FLUID_DBZ_" + re.sub(r"[^A-Za-z0-9]", "_", key.removeprefix("debezium.")).upper() if env.get(name, text) != text: raise ValueError(f"two debezium properties map to the environment variable {name}") @@ -543,7 +571,9 @@ def line(key: str, value: object) -> str: lines = [line(f"debezium.source.{k}", v) for k, v in source_config.items()] lines.append("quarkus.log.console.json=false") lines.append(_properties_line("debezium.sink.type", sink_type)) - lines += [line(f"debezium.sink.{sink_type}.{k}", v) for k, v in sink_config.items()] + lines += [ + line(f"debezium.sink.{sink_type}.{k}", v, env_refs=True) for k, v in sink_config.items() + ] return "\n".join(lines), env @@ -638,14 +668,13 @@ def _execute_debezium_server( ) except ValueError as exc: return _failed(ctx, started_at, t_start, str(exc)) - from ...credentials.encrypted_store import _atomic_write_bytes - - # Created at 0o600 (O_EXCL, then an atomic rename): never briefly - # world-readable, and no chmod whose failure could be ignored. A - # hand-written sink key the credential rule does not name still lands - # in this file. + # Created 0o600 and tightened to 0o600 when it already exists (a copy an + # earlier release wrote at 0o644 with the password in it is truncated + # first): never world-readable, and no chmod whose failure is ignored. A + # symlink in its place is refused rather than followed. A hand-written + # sink key the credential rule does not name still lands in this file. try: - _atomic_write_bytes(config_path, properties.encode("utf-8"), mode=0o600) + _write_private_text(config_path, properties) except OSError as exc: return _failed(ctx, started_at, t_start, f"could not write {config_path}: {exc}") 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 8b4c30e5..9c9a2ffa 100644 --- a/fluid_build/build_runners/kafka_connect/iceberg_sink_validation.py +++ b/fluid_build/build_runners/kafka_connect/iceberg_sink_validation.py @@ -1261,7 +1261,8 @@ def _check_inlined_secrets( """An override map that writes a literal ``credential`` / ``token``. WARNING. The value is inlined into the config forge-cli pushes to the Connect REST - API or writes to ``application.properties``. Never quotes the value. A key + API or passes to Debezium Server (in its environment, not in + ``application.properties``). Never quotes the value. A key the catalogAuth conflict check already refuses is not repeated here. """ prefix = _property_prefix(runtime) @@ -1271,7 +1272,7 @@ def _check_inlined_secrets( and runtime.engine in _CATALOG_AUTH_ENGINES ) where = ( - "writes to application.properties" + "passes to Debezium Server" if runtime.engine == "debezium" else "pushes to the Kafka Connect REST API" ) diff --git a/fluid_build/build_runners/meltano/runner.py b/fluid_build/build_runners/meltano/runner.py index 4053514b..a1c4d590 100644 --- a/fluid_build/build_runners/meltano/runner.py +++ b/fluid_build/build_runners/meltano/runner.py @@ -181,24 +181,27 @@ def _resolve_tap_binary(tap_name: str, *, project_dir: Optional[Path] = None) -> @contextmanager -def _private_tap_config(config: Dict[str, Any], workdir: Path) -> Iterator[Path]: - """Yield the path of a tap's ``--config`` file that only this user can read. +def _private_singer_config(config: Dict[str, Any], workdir: Path, name: str) -> Iterator[Path]: + """Yield the path of a Singer tap's or target's ``--config`` file that only + this user can read. Singer's ``--config`` contract puts the resolved credential in a file: it - is how a tap reads its password. The file is created 0o600 inside a - 0o700 ``TemporaryDirectory`` (the pattern ``dbt/profiles.py`` uses for a - generated ``profiles.yml``), and removed when the tap exits, however it - exits. It used to be ``/tap_config.json``: under the contract - directory, 0o644, never deleted, and not in the ``.gitignore`` that - ``fluid init`` writes, so ``git add -A`` staged the password. A copy an - earlier release left there is deleted. + is how a tap or target reads its password. The file is created 0o600 + inside a 0o700 ``TemporaryDirectory`` (the pattern ``dbt/profiles.py`` + uses for a generated ``profiles.yml``) and removed when the ``with`` + block exits: after the subprocess returns, fails or times out. A fluid + process killed by a signal can leave it in that private directory. It + used to be ``/``: under the contract directory, 0o644, + never deleted, and not in the ``.gitignore`` that ``fluid init`` writes, + so ``git add -A`` staged the password. A copy an earlier release left + there is deleted. A tap that writes a refreshed OAuth token back to its config file loses that write; the next run resolves the credential from ``secretRef`` again. """ - (workdir / "tap_config.json").unlink(missing_ok=True) + (workdir / name).unlink(missing_ok=True) with tempfile.TemporaryDirectory(prefix="fluid-singer-") as tmp: - path = Path(tmp) / "tap_config.json" + path = Path(tmp) / name fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) with os.fdopen(fd, "w", encoding="utf-8") as fh: json.dump(config, fh) @@ -227,7 +230,7 @@ def invoke_tap( # regardless of its cwd. Using relative paths here would have the # tap look for ``//tap_state.json`` after subprocess # cd's into ``workdir`` — that's a double-prefix bug we hit. - with _private_tap_config(config, workdir) as config_path: + with _private_singer_config(config, workdir, "tap_config.json") as config_path: cmd = [binary, "--config", str(config_path)] if state is not None: state_path = (workdir / "tap_state.json").resolve() @@ -271,7 +274,7 @@ def discover_tap_catalog( """ workdir.mkdir(parents=True, exist_ok=True) LOG.info("singer.discover binary=%s", binary) - with _private_tap_config(config, workdir) as config_path: + with _private_singer_config(config, workdir, "tap_config.json") as config_path: cmd = [binary, "--config", str(config_path), "--discover"] try: proc = subprocess.run( @@ -447,9 +450,6 @@ def invoke_target( code and surfaces stderr to the operator. """ workdir.mkdir(parents=True, exist_ok=True) - config_path = (workdir / "target_config.json").resolve() - config_path.write_text(json.dumps(config), encoding="utf-8") - cmd = [binary, "--config", str(config_path)] LOG.info("singer.target.invoke binary=%s cwd=%s", binary, workdir) # Pre-render the Singer message stream once; Popen.communicate(input=...) @@ -465,20 +465,21 @@ def invoke_target( lines.append(json.dumps({"type": "STATE", "value": state})) singer_input = "\n".join(lines) + ("\n" if lines else "") - proc = subprocess.Popen( - cmd, - cwd=str(workdir.resolve()), - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - text=True, - ) - try: - _stdout, stderr = proc.communicate(input=singer_input, timeout=timeout_seconds) - except subprocess.TimeoutExpired: - proc.kill() - _stdout, stderr = proc.communicate() - return {"exit_code": -1, "stderr": f"target timeout after {timeout_seconds}s\n{stderr}"} + with _private_singer_config(config, workdir, "target_config.json") as config_path: + proc = subprocess.Popen( + [binary, "--config", str(config_path)], + cwd=str(workdir.resolve()), + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + try: + _stdout, stderr = proc.communicate(input=singer_input, timeout=timeout_seconds) + except subprocess.TimeoutExpired: + proc.kill() + _stdout, stderr = proc.communicate() + return {"exit_code": -1, "stderr": f"target timeout after {timeout_seconds}s\n{stderr}"} # Persist stderr to disk so an operator can re-read after the run # without re-piping. Mirrors the tap-side ``tap_discover_stderr.log`` diff --git a/fluid_build/cli/init.py b/fluid_build/cli/init.py index efe1e96f..62c48e8f 100644 --- a/fluid_build/cli/init.py +++ b/fluid_build/cli/init.py @@ -808,9 +808,9 @@ def _ensure_workspace(args, logger: logging.Logger) -> None: .fluid/copilot-memory.json .fluid/logs/ -# Generated per-run connector config (never commit; regenerated each run) -.fluid/debezium/ -.fluid/meltano/ +# Generated per-run connector config, in every product directory (never commit) +**/.fluid/debezium/ +**/.fluid/meltano/ # Build outputs (never commit) runtime/ diff --git a/fluid_build/copilot/missions/checks.py b/fluid_build/copilot/missions/checks.py index 5563bca9..6382ac3c 100644 --- a/fluid_build/copilot/missions/checks.py +++ b/fluid_build/copilot/missions/checks.py @@ -347,20 +347,24 @@ def _display_leaf(concrete: str, leaf: Any) -> Any: reads ``password: expected == …, got 'x'`` as a ``password`` assignment whose value is the word ``expected``, so ``'x'`` reached the terminal, ``--json``, the scorecard file and the planner's LLM prompt. Mask it here - instead, by the path's key names, with the semantics ``redact_value`` - gives a mapping key: a leaf at or under a credential-named key is never - shown, and any other leaf goes through ``redact_value``. + instead, by the path's key names: a leaf at or under a key that names a + credential (:func:`names_a_credential`) is never shown, and any other leaf + goes through ``redact_value``. A value that cannot be redacted (a YAML + alias that refers to itself) is not shown either. """ from fluid_build.observability.secret_redactor import ( _REDACTED, - is_sensitive_key_name, + names_a_credential, redact_value, ) keys = [s for s in re.split(r"[.\[\]]", concrete) if s and s != "*" and not s.isdigit()] - if any(is_sensitive_key_name(key) for key in keys): + if any(names_a_credential(key) for key in keys): + return _REDACTED + try: + return redact_value(leaf) + except RecursionError: return _REDACTED - return redact_value(leaf) def _compare(op: str, leaf: Any, expected: Any, *, shown: Any) -> Tuple[bool, str]: diff --git a/fluid_build/observability/secret_redactor.py b/fluid_build/observability/secret_redactor.py index fb536b1f..37ae72be 100644 --- a/fluid_build/observability/secret_redactor.py +++ b/fluid_build/observability/secret_redactor.py @@ -50,6 +50,15 @@ from collections.abc import Iterable, Mapping from typing import Any +# The key names whose ``key=value`` / ``key: value`` VALUE is a credential: one +# vocabulary for the assignment rule below, the placeholder rule, and +# :func:`names_a_credential`. +_CREDENTIAL_KEY_WORDS = ( + r"api[_-]?key|authorization|aws_secret_access_key|secret[_.-]access[_.-]key|secret[_.-]key|" + r"client_secret|credentials?|oauth[_-]?token|password|passphrase|" + r"private[_-]?key(?:_passphrase)?|session[_-]token|secret|token" +) + _REDACTED = "***REDACTED***" # Substring-match list: a mapping/env key is sensitive when any entry is # a substring of the lower-cased key name. Keep alphabetically sorted. @@ -194,11 +203,7 @@ # Pinned by ``tests/security/test_security_regressions_round3.py``. _ASSIGNMENT_RE = re.compile( r"(?ix)" - r"(?P\b(?:[A-Za-z0-9_]{,128}_)?(?:" - r"api[_-]?key|authorization|aws_secret_access_key|secret[_.-]access[_.-]key|secret[_.-]key|" - r"client_secret|credentials?|oauth[_-]?token|password|passphrase|" - r"private[_-]?key(?:_passphrase)?|session[_-]token|secret|token" - r")\b)" + r"(?P\b(?:[A-Za-z0-9_]{,128}_)?(?:" + _CREDENTIAL_KEY_WORDS + r")\b)" r"(?P['\"]?\s{,8}[:=]\s{,8})" r"(?P['\"]?)" r"(?P.{,256}?)(?P=quote)" @@ -228,11 +233,7 @@ # inspect the last ~64 characters before each placeholder so the regex stays # linear in the message length. _PRECEDING_SENSITIVE_KEY_RE = re.compile( - r"(?ix)\b(?:[A-Za-z0-9_]*_)?(?:" - r"api[_-]?key|authorization|aws_secret_access_key|secret[_.-]access[_.-]key|secret[_.-]key|" - r"client_secret|credentials?|oauth[_-]?token|password|passphrase|" - r"private[_-]?key(?:_passphrase)?|session[_-]token|secret|token" - r")\b\s*[:=]\s*$" + r"(?ix)\b(?:[A-Za-z0-9_]*_)?(?:" + _CREDENTIAL_KEY_WORDS + r")\b\s*[:=]\s*$" ) @@ -633,6 +634,26 @@ def is_sensitive_key_name(name: str) -> bool: return _is_sensitive_key(name) +_CAMEL_BOUNDARY_RE = re.compile(r"(?<=[a-z0-9])(?=[A-Z])") +_NAMES_A_CREDENTIAL_RE = re.compile(r"(?i)(?:^|_)(?:" + _CREDENTIAL_KEY_WORDS + r")$") + + +def names_a_credential(name: str) -> bool: + """True when the key ``name`` itself names a credential, as the text + layer's ``key=value`` rule reads one: ``db_password``, ``apiKey``, + ``private-key`` and ``clientSecret`` do; ``maxTokensPerDay``, + ``secretRef``, ``tokenEnv``, ``authz`` and ``author`` do not. + + camelCase, ``-`` and ``.`` read as ``_``, and the name must END with a + credential word. Narrower than :func:`is_sensitive_key_name`, which + matches a sensitive word anywhere in a key: right for masking a logging + payload, too broad where hiding a value costs the reader, such as a + diagnostic that says what a field holds. + """ + snake = _CAMEL_BOUNDARY_RE.sub("_", name).replace("-", "_").replace(".", "_") + return bool(_NAMES_A_CREDENTIAL_RE.search(snake)) + + def redact_value(value: Any) -> Any: """Recursively redact secret-like values in logging payloads.""" if isinstance(value, str): diff --git a/fluid_build/providers/local/util/logging.py b/fluid_build/providers/local/util/logging.py index 395ed553..4871c3e1 100644 --- a/fluid_build/providers/local/util/logging.py +++ b/fluid_build/providers/local/util/logging.py @@ -23,7 +23,11 @@ import re from typing import Any, Dict, List -from fluid_build.observability.secret_redactor import is_sensitive_key_name, redact_secret_text +from fluid_build.observability.secret_redactor import ( + _URL_USERINFO_RE, + is_sensitive_key_name, + redact_known_values, +) # Patterns for sensitive data that should be redacted SENSITIVE_PATTERNS = [ @@ -85,16 +89,18 @@ def redact_string(text: str) -> str: if not isinstance(text, str): return text - redacted = text + # The two layers the Snowflake twin runs first: the credentials this + # process holds, masked whole (before a pattern below can cut one short), + # then any ``scheme://user:password@`` userinfo. ``_log_error`` passes a # pragma: allowlist secret + # raw ``str(e)`` through here into the record's ``ctx``, which a JSON log + # formatter renders, and the patterns below knew only ``postgresql://``. + redacted = redact_known_values(text, placeholder="[REDACTED]") + redacted = _URL_USERINFO_RE.sub(r"\1[REDACTED]\2", redacted) for pattern in SENSITIVE_PATTERNS: redacted = pattern.sub("[REDACTED]", redacted) - # Then the shared redactor: the patterns above are a provider-local - # subset (no ``postgres://`` userinfo, no ``token=``, no credential this - # process registered), and ``_log_error`` passes a raw ``str(e)`` through - # here into the record's ``ctx``, which a JSON log formatter renders. - return redact_secret_text(redacted) + return redacted def redact_dict(data: Dict[str, Any], max_depth: int = 10) -> Dict[str, Any]: diff --git a/fluid_build/providers/snowflake/util/logging.py b/fluid_build/providers/snowflake/util/logging.py index cee1b954..b008e75e 100644 --- a/fluid_build/providers/snowflake/util/logging.py +++ b/fluid_build/providers/snowflake/util/logging.py @@ -100,7 +100,7 @@ # Including it HERE would run before the PEM-block pattern and # corrupt a ``private_key=`` header, leaking the body. r"(?i)\b(?P(?:[A-Za-z0-9_]{,128}_)?(?:" - r"api[_-]?key|authorization|secret[_-]access[_-]key|client_secret|" + r"api[_-]?key|authorization|secret[_.-]access[_.-]key|secret[_.-]key|client_secret|" r"oauth[_-]?token|password|session[_-]token|secret|token|passphrase" r"))" r"(?P\s{,8}[:=]\s{,8})" diff --git a/tests/build_runners/test_catalog_auth_validation.py b/tests/build_runners/test_catalog_auth_validation.py index 20b92beb..aa77af3b 100644 --- a/tests/build_runners/test_catalog_auth_validation.py +++ b/tests/build_runners/test_catalog_auth_validation.py @@ -298,7 +298,7 @@ def test_a_literal_secret_in_a_handwritten_config_warns(): warnings = validate_iceberg_sink(contract)[1] assert any( "debezium.server.sink.config sets credential to a literal value" in w - and "writes to application.properties" in w + and "passes to Debezium Server" in w and "id:sec" not in w for w in warnings ), warnings diff --git a/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py b/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py index c0f1607e..2b7276ba 100644 --- a/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py +++ b/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py @@ -53,6 +53,8 @@ _S3_KEY = "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY" # pragma: allowlist secret _NEWLINE_VALUE = "a\nb" # pragma: allowlist secret _SECRET_REF = "env://PG_PASSWORD_T211" # pragma: allowlist secret +_ENV_PREFIXED = "env:" + _SECRET +_UC_TOKEN_REF = "env:UC_TOKEN" # pragma: allowlist secret # ── rename: the redactor's output is not a CodeQL "secret" source ───────── @@ -205,6 +207,63 @@ def test_an_ordinary_leaf_is_still_quoted(tmp_path): assert "got 'sales'" in diagnostics +def _diagnostic(path: str, leaf, op="eq", value="x") -> str: + from fluid_build.copilot.missions.checks import _check_predicate, _redact_result + from fluid_build.copilot.missions.spec import CriterionSpec + + contract: dict = {} + node = contract + keys = path.split(".") + for key in keys[:-1]: + node = node.setdefault(key, {}) + node[keys[-1]] = leaf + result = _redact_result( + _check_predicate( + CriterionSpec(check="predicate", path=path, op=op, value=value, value_provided=True), + contract, + contract_path=Path("c.fluid.yaml"), + ) + ) + return " ".join(result.diagnostics) + + +@pytest.mark.parametrize( + "path", ["a.privateKey", "a.private-key", "a.apiKey", "a.clientSecret", "a.sessionToken"] +) +def test_a_camel_or_kebab_case_credential_key_is_masked(path): + assert _SECRET not in _diagnostic(path, _SECRET) + + +@pytest.mark.parametrize( + "path, leaf, op, value, shown", + [ + ("policy.agentPolicy.maxTokensPerDay", 500000, "lte", 100000, "got 500000"), + ("policy.authz.readers", ["group:everyone"], "contains", "group:x", "'group:everyone'"), + ("binding.secretRef.provider", "vault", "eq", "env", "got 'vault'"), + ("sink.catalogAuth.tokenEnv", "UC_TOKEN", "eq", "X", "got 'UC_TOKEN'"), + ("metadata.author", "alice", "eq", "bob", "got 'alice'"), + ], +) +def test_a_key_that_only_mentions_a_credential_word_is_shown(path, leaf, op, value, shown): + """``auth``/``token``/``secret`` inside a longer key name is not a credential: + the diagnostic must keep the value the planner needs to repair it.""" + assert shown in _diagnostic(path, leaf, op, value) + + +def test_a_self_referential_leaf_does_not_flip_the_predicate(tmp_path): + from fluid_build.copilot.missions.checks import _check_predicate + from fluid_build.copilot.missions.spec import CriterionSpec + + cyclic: dict = {"x": 1} + cyclic["self"] = cyclic # a YAML alias that refers to itself + result = _check_predicate( + CriterionSpec(check="predicate", path="a.s", op="ne", value={"x": 2}, value_provided=True), + {"a": {"s": cyclic}}, + contract_path=Path("c.fluid.yaml"), + ) + assert result.passed + + # ── #292: the local provider's log context ─────────────────────────────── @@ -237,6 +296,32 @@ def test_local_redact_sql_masks_a_postgres_url_password(): assert _SECRET not in out +def test_local_redaction_masks_a_held_secret_whole_before_its_patterns(monkeypatch): + """A held secret with ``;``/``@`` is masked whole: the local + ``password=[^\\s;&]+`` pattern must not cut it short first.""" + from fluid_build.providers.local.util.logging import redact_string + + held = "Kq7;zR9@xWv2-held" # pragma: allowlist secret + monkeypatch.setattr(sr, "_known_secrets", (held,)) + out = redact_string(f"could not connect: password={held} host=db") + assert "zR9" not in out and "xWv2" not in out + + +def test_local_redaction_keeps_dotted_table_names(): + from fluid_build.providers.local.util.logging import redact_dict, redact_sql + + table = "analytics_prod.customer_orders.order_items_daily" + assert redact_dict({"table": table})["table"] == table + assert table in redact_sql(f"SELECT * FROM {table} WHERE x=1") + + +def test_snowflake_twin_masks_the_dotted_spellings_too(): + from fluid_build.providers.snowflake.util.logging import redact_string + + for key in ("aws.secret.access.key", "fs.s3a.secret.key", "secret_key"): + assert _S3_KEY not in redact_string(f"{key}={_S3_KEY}") + + # ── #211: Debezium Server gets its credentials from the environment ────── @@ -274,8 +359,39 @@ def test_credential_properties_render_as_env_expressions(): def test_a_control_character_in_a_credential_still_fails_closed(): from fluid_build.build_runners.debezium.runner import render_application_properties - with pytest.raises(ValueError, match="control character"): + with pytest.raises(ValueError, match="control character") as excinfo: render_application_properties({"database.password": _NEWLINE_VALUE}, "kafka", {}) + assert "TAIL" not in str(excinfo.value) and "a\\nb" not in str(excinfo.value) + + +def test_env_prefix_is_iceberg_syntax_for_the_sink_only(): + from fluid_build.build_runners.debezium.runner import render_application_properties + + text, env = render_application_properties( + {"database.password": _ENV_PREFIXED}, "iceberg", {"token": _UC_TOKEN_REF} + ) + assert _SECRET not in text + assert env == {"FLUID_DBZ_SOURCE_DATABASE_PASSWORD": _ENV_PREFIXED} + assert "debezium.sink.iceberg.token=env:UC_TOKEN" in text + + +@pytest.mark.parametrize( + "key", + [ + "connectionstring", + "header.Authorization", + "client.authParams", + "adls.auth.shared-key.account.key", + "s3.sse.key", + "producer.ssl.keystore.key", + ], +) +def test_more_credential_property_names_go_to_the_environment(key): + from fluid_build.build_runners.debezium.runner import render_application_properties + + text, env = render_application_properties({}, "eventhubs", {key: _SECRET}) + assert _SECRET not in text + assert list(env.values()) == [_SECRET] def test_two_properties_on_one_env_name_fail_closed(): @@ -325,15 +441,8 @@ def test_embedded_server_file_holds_no_password_and_server_gets_it(tmp_path, mon assert captured.read_text() == _SECRET -def test_an_unwritable_config_is_a_failed_run_not_a_crash(tmp_path, monkeypatch): - from fluid_build.build_runners.debezium import runner as dbz - from fluid_build.credentials import encrypted_store - - def refuse(*_a, **_k): - raise FileExistsError(17, "File exists") - - monkeypatch.setattr(encrypted_store, "_atomic_write_bytes", refuse) - ctx = SimpleNamespace( +def _embedded_ctx(tmp_path): + return SimpleNamespace( workdir=str(tmp_path), product_id="p", build_id="b", @@ -345,8 +454,13 @@ def refuse(*_a, **_k): sink=None, env={}, ) - result = dbz._execute_debezium_server( - ctx, + + +def _run_embedded(tmp_path): + from fluid_build.build_runners.debezium import runner as dbz + + return dbz._execute_debezium_server( + _embedded_ctx(tmp_path), {"engine": "debezium", "properties": {}}, {"mode": "embedded"}, {"server": {"sink": {"type": "kafka"}}}, @@ -354,10 +468,43 @@ def refuse(*_a, **_k): "2026-10-13T00:00:00Z", 0.0, ) + + +def test_an_unwritable_config_is_a_failed_run_not_a_crash(tmp_path, monkeypatch): + from fluid_build.build_runners.debezium import runner as dbz + + def refuse(*_a, **_k): + raise PermissionError(13, "Permission denied") + + monkeypatch.setattr(dbz, "_write_private_text", refuse) + result = _run_embedded(tmp_path) assert result.state.value == "failed" assert "could not write" in (result.error or "") +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX file modes") +def test_an_earlier_releases_world_readable_config_is_truncated_and_tightened(tmp_path): + config = tmp_path / ".fluid/debezium/p/b/application.properties" + config.parent.mkdir(parents=True) + config.write_text(f"debezium.source.database.password={_SECRET}\n") + config.chmod(0o644) + _run_embedded(tmp_path) + assert _SECRET not in config.read_text() + assert stat.S_IMODE(config.stat().st_mode) == 0o600 + + +@pytest.mark.skipif(sys.platform == "win32", reason="symlinks and O_NOFOLLOW") +def test_a_symlink_in_place_of_the_config_is_refused(tmp_path): + target = tmp_path / "elsewhere.txt" + target.write_text("untouched") + config = tmp_path / ".fluid/debezium/p/b/application.properties" + config.parent.mkdir(parents=True) + config.symlink_to(target) + result = _run_embedded(tmp_path) + assert result.state.value == "failed" + assert target.read_text() == "untouched" + + # ── #53 / #54: the Singer tap config is private and short-lived ────────── @@ -372,8 +519,8 @@ def _fake_tap(tmp_path: Path, exit_code: int, sleep: float = 0.0) -> Path: "with open(path) as fh: body = fh.read()\n" "mode = stat.S_IMODE(os.stat(path).st_mode)\n" "parent = stat.S_IMODE(os.stat(os.path.dirname(path)).st_mode)\n" - f"json.dump({{'path': path, 'mode': mode, 'parent': parent, 'body': body}}," - f" open({str(seen)!r}, 'w'))\n" + f"with open({str(seen)!r}, 'w') as out:\n" + " json.dump({'path': path, 'mode': mode, 'parent': parent, 'body': body}, out)\n" f"time.sleep({sleep})\n" f"sys.exit({exit_code})\n" ) @@ -423,10 +570,10 @@ def test_discover_removes_the_config_after_a_failed_tap(tmp_path): def test_discover_removes_the_config_after_a_timeout(tmp_path): from fluid_build.build_runners.meltano.runner import discover_tap_catalog - tap = _fake_tap(tmp_path, 0, sleep=30) + tap = _fake_tap(tmp_path, 0, sleep=120) assert ( discover_tap_catalog( - str(tap), config=_tap_config(), workdir=tmp_path / "w", timeout_seconds=2 + str(tap), config=_tap_config(), workdir=tmp_path / "w", timeout_seconds=8 ) is None ) @@ -434,6 +581,26 @@ def test_discover_removes_the_config_after_a_timeout(tmp_path): assert not Path(seen["path"]).exists() +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX file modes and a script target") +def test_invoke_target_config_is_private_and_removed(tmp_path): + from fluid_build.build_runners.meltano.runner import invoke_target + + workdir = tmp_path / ".fluid/meltano/p/b" + workdir.mkdir(parents=True) + stale = workdir / "target_config.json" + stale.write_text(json.dumps(_tap_config())) # an earlier release's copy + + invoke_target( + str(_fake_tap(tmp_path, 0)), config=_tap_config(), schemas={}, records={}, workdir=workdir + ) + + seen = json.loads((tmp_path / "tap-saw.json").read_text()) + assert json.loads(seen["body"])["password"] == _SECRET + assert seen["mode"] == 0o600 and seen["parent"] == 0o700 + assert not Path(seen["path"]).exists() + assert not stale.exists() + + # ── fluid init: generated connector config is git-ignored ──────────────── @@ -446,6 +613,9 @@ def test_init_gitignore_block_ignores_generated_connector_config(tmp_path): for rel in ( ".fluid/debezium/sales.orders/b1/application.properties", ".fluid/meltano/sales.orders/b1/tap_state.json", + # each product lives in its own directory under the workspace root + "customer-360/.fluid/debezium/sales.orders/b1/application.properties", + "customer-360/.fluid/meltano/sales.orders/b1/target_stderr.log", ): result = subprocess.run(["git", "-C", str(tmp_path), "check-ignore", "-q", rel]) assert result.returncode == 0, rel From 915dc377af69c504911f4547ed1c1c88c0ffec61 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Tue, 13 Oct 2026 22:50:48 +0200 Subject: [PATCH 4/6] fix(security): mission masking fails closed; local exception text gets the shared redactor The second security review found that d1b79106 traded leaks for precision: - fluid mission: names_a_credential (end-anchored) showed values under connection_string, conn_str, auth, bearer, secrets, tokens, *Value, PGPASSWORD and password1, which fa8aa5e8 masked. The display rule is now the shared is_sensitive_key_name (fails closed) with three explicit exceptions a repairing planner needs: a key whose last word makes it a reference (secretRef, tokenEnv, catalogAuth.type, passwordFile), words that only contain "auth" (authz, author), and token budgets (maxTokensPerDay); plus passwd/pwd/dsn/connectionstring the shared rule lacks. names_a_credential is removed. - local provider: redact_string keeps the two layers (held values whole, URL userinfo) before its patterns, so table names stay readable and long SQL stays linear; the exception text it logs at the five raw str(e) sites now goes through redact_secret_text, as local.py:506 already did (token=, api_key=, client_secret=, Bearer, AKIA, PEM, JWT). Tests: 87 in the security file (26 fail on d1b79106); the Debezium renderer is pinned against catalogAuth's secret property names, and the embedded server is shown to inherit the environment as well as receive FLUID_DBZ_*. CHANGELOG: the #712 note, #231 and #292 wording corrected. --- .secrets.baseline | 4 +- CHANGELOG.md | 29 ++--- fluid_build/build_runners/meltano/runner.py | 4 +- fluid_build/copilot/missions/checks.py | 64 ++++++++-- fluid_build/observability/secret_redactor.py | 23 +--- fluid_build/providers/local/local.py | 16 ++- fluid_build/providers/local/util/logging.py | 7 +- ...tials_stay_out_of_files_and_diagnostics.py | 114 ++++++++++++++++-- 8 files changed, 195 insertions(+), 66 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 821f75f0..97c28e25 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -427,7 +427,7 @@ "filename": "fluid_build/observability/secret_redactor.py", "hashed_secret": "1348b145fa1a555461c1b790a2f66614781091e9", "is_verified": false, - "line_number": 165 + "line_number": 164 } ], "fluid_build/providers/_sql_safety.py": [ @@ -2154,5 +2154,5 @@ } ] }, - "generated_at": "2026-10-13T20:32:20Z" + "generated_at": "2026-10-13T20:49:43Z" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f6fe089..84390049 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,7 +109,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `fluid validate` and the streaming runners now warn when an Iceberg sink override (`iceberg_catalog_overrides`, `sink_connector_config`, `debezium.server.sink.config`) sets the catalog's `credential` or `token` to a literal value, which forge-cli then pushes to - the Kafka Connect REST API or writes to `application.properties`. A value that starts + the Kafka Connect REST API or passes to Debezium Server. A value that starts with `env:` or carries a `${...}` expression does not warn. Under `fluid validate --strict` the warning fails validation; use `sink.catalogAuth`, or an `env:` value. @@ -449,19 +449,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 predicate on a path such as `exposes[*].binding.location.password` printed `got ''` to the terminal, `--json`, the stored scorecard and the planner's LLM prompt, because the redactor read `password: expected == …` as the assignment. A leaf at - or under a key that names a credential (`password`, `apiKey`, `privateKey`, - `clientSecret`, `sessionToken`, …) now shows as `***REDACTED***`; a key that only - contains such a word (`maxTokensPerDay`, `secretRef`, `tokenEnv`) still shows its value. - (CodeQL alert 241.) -- **The shared redactor masks `aws.secret.access.key=`, `secret.access.key=` and - `fs.s3a.secret.key=`.** It knew only the `_` and `-` spellings, so a Kafka Connect task - trace stored in a run record kept an S3 secret key. The Snowflake provider's redactor - masks the same spellings. (CodeQL alert 231.) -- **The local provider's log context masks the credentials forge-cli holds and URL - userinfo.** A failed SQL action put the raw error into the log record's context, which a - JSON log formatter renders, and the provider's own patterns knew only `postgresql://`. - It now runs the same two layers first that the Snowflake provider does: each registered - credential masked whole, then any `scheme://user:password@`. (CodeQL alert 292.) + or under a key the redactor treats as a credential (`password`, `secrets`, `auth`, + `connection_string`, `apiKey`, `PGPASSWORD`, …) now shows as `***REDACTED***`. A key that + refers to a credential (`secretRef`, `tokenEnv`, `catalogAuth.type`) or only contains such + a word (`maxTokensPerDay`, `authz`, `author`) still shows its value. (CodeQL alert 241.) +- **The shared redactor masks `aws.secret.access.key=`, `secret.access.key=`, + `fs.s3a.secret.key=` and `secret_key=`.** It knew `secret_access_key` with `_` or `-`, and + no `secret.key` form at all, so a Kafka Connect task trace stored in a run record kept an + S3 secret key. The Snowflake provider's redactor masks the same spellings. (CodeQL alert + 231.) +- **The local provider's log context no longer carries a raw error.** A failed plan, load, + SQL or copy action put the exception's text into the log record's context, which a JSON + log formatter renders. That text now goes through the shared redactor, and every context + value first gets the two layers the Snowflake provider runs: each credential forge-cli + holds masked whole, then any `scheme://user:password@` userinfo. (CodeQL alert 292.) - `mask_known_secrets` is now `redact_known_values`; the old name stays as an alias. CodeQL treats a call to a function named like `*secret*` as a source of secrets, so every caller of the redactor was reported as logging or storing a credential it had already masked. diff --git a/fluid_build/build_runners/meltano/runner.py b/fluid_build/build_runners/meltano/runner.py index a1c4d590..25dc1056 100644 --- a/fluid_build/build_runners/meltano/runner.py +++ b/fluid_build/build_runners/meltano/runner.py @@ -187,8 +187,8 @@ def _private_singer_config(config: Dict[str, Any], workdir: Path, name: str) -> Singer's ``--config`` contract puts the resolved credential in a file: it is how a tap or target reads its password. The file is created 0o600 - inside a 0o700 ``TemporaryDirectory`` (the pattern ``dbt/profiles.py`` - uses for a generated ``profiles.yml``) and removed when the ``with`` + inside a 0o700 ``TemporaryDirectory`` (as ``dbt/profiles.py`` keeps a + generated ``profiles.yml``) and removed when the ``with`` block exits: after the subprocess returns, fails or times out. A fluid process killed by a signal can leave it in that private directory. It used to be ``/``: under the contract directory, 0o644, diff --git a/fluid_build/copilot/missions/checks.py b/fluid_build/copilot/missions/checks.py index 6382ac3c..7b6a7c0d 100644 --- a/fluid_build/copilot/missions/checks.py +++ b/fluid_build/copilot/missions/checks.py @@ -340,6 +340,51 @@ def _is_number(value: Any) -> bool: return isinstance(value, (int, float)) and not isinstance(value, bool) +#: A key whose last word makes it a REFERENCE to a credential, not one: +#: ``secretRef``, ``tokenEnv``, ``credentialEnv``, ``catalogAuth.type``, +#: ``oauth2ServerUri``, ``passwordFile``. +_REFERENCE_WORDS = frozenset( + {"ref", "env", "type", "kind", "mode", "provider", "uri", "url", "endpoint"} + | {"scope", "audience", "resource", "name", "file", "path", "enabled"} +) +#: Words that only contain a sensitive part (``auth``): ``authz``, ``author``. +_NOT_CREDENTIAL_WORDS = frozenset({"authz", "authn", "author", "authors", "authority"}) +#: A token budget, not a token: ``maxTokensPerDay``, ``tokensPerMinute``. +_TOKEN_BUDGET_RE = re.compile(r"(?:^|_)(?:max|min)_tokens(?:_|$)|(?:^|_)tokens_per(?:_|$)") +#: Spellings the shared key rule does not know: ``MYSQL_PWD``, ``passwd``, ``dsn``. +_MORE_CREDENTIAL_NAMES_RE = re.compile(r"(?:passwd|pwd|dsn|connectionstring)$") +_KEY_WORD_BREAK_RE = re.compile(r"[^A-Za-z0-9]+|(?<=[a-z0-9])(?=[A-Z])") + + +def _key_words(key: str) -> List[str]: + return [word.lower() for word in _KEY_WORD_BREAK_RE.split(key) if word] + + +def _key_hides_its_value(key: str) -> bool: + """Whether a contract key holds a credential, so a diagnostic must not + quote what is at or under it. + + The shared rule (``is_sensitive_key_name``, which masks a mapping key in + every logging payload) decides, so ``passwords``, ``auth``, + ``connection_string``, ``secretValue`` and ``PGPASSWORD`` hide their value. + Three kinds of key are read as what they are instead: one that refers to + a credential (its last word is in ``_REFERENCE_WORDS``), a word that only + contains ``auth`` (``authz``, ``author``), and a token budget + (``maxTokensPerDay``). A planner repairing those needs the value. + """ + from fluid_build.observability.secret_redactor import is_sensitive_key_name + + words = _key_words(key) + if not words or words[-1] in _REFERENCE_WORDS: + return False + name = "_".join(word for word in words if word not in _NOT_CREDENTIAL_WORDS) + if not name or _TOKEN_BUDGET_RE.search(name): + return False + return is_sensitive_key_name(name) or bool( + _MORE_CREDENTIAL_NAMES_RE.search(name.replace("_", "")) + ) + + def _display_leaf(concrete: str, leaf: Any) -> Any: """The leaf as a diagnostic may quote it (``got ``). @@ -347,19 +392,18 @@ def _display_leaf(concrete: str, leaf: Any) -> Any: reads ``password: expected == …, got 'x'`` as a ``password`` assignment whose value is the word ``expected``, so ``'x'`` reached the terminal, ``--json``, the scorecard file and the planner's LLM prompt. Mask it here - instead, by the path's key names: a leaf at or under a key that names a - credential (:func:`names_a_credential`) is never shown, and any other leaf - goes through ``redact_value``. A value that cannot be redacted (a YAML - alias that refers to itself) is not shown either. + instead, by the path's key names: a leaf at or under a key that holds a + credential (:func:`_key_hides_its_value`) is never shown, unless the leaf's + own key refers to one (``catalogAuth.tokenEnv``), and any other leaf goes + through ``redact_value``. A value that cannot be redacted (a YAML alias + that refers to itself) is not shown either. """ - from fluid_build.observability.secret_redactor import ( - _REDACTED, - names_a_credential, - redact_value, - ) + from fluid_build.observability.secret_redactor import _REDACTED, redact_value keys = [s for s in re.split(r"[.\[\]]", concrete) if s and s != "*" and not s.isdigit()] - if any(names_a_credential(key) for key in keys): + leaf_words = _key_words(keys[-1]) if keys else [] + leaf_is_a_reference = bool(leaf_words) and leaf_words[-1] in _REFERENCE_WORDS + if not leaf_is_a_reference and any(_key_hides_its_value(key) for key in keys): return _REDACTED try: return redact_value(leaf) diff --git a/fluid_build/observability/secret_redactor.py b/fluid_build/observability/secret_redactor.py index 37ae72be..e6eed3b2 100644 --- a/fluid_build/observability/secret_redactor.py +++ b/fluid_build/observability/secret_redactor.py @@ -51,8 +51,7 @@ from typing import Any # The key names whose ``key=value`` / ``key: value`` VALUE is a credential: one -# vocabulary for the assignment rule below, the placeholder rule, and -# :func:`names_a_credential`. +# vocabulary for the assignment rule and the placeholder rule below. _CREDENTIAL_KEY_WORDS = ( r"api[_-]?key|authorization|aws_secret_access_key|secret[_.-]access[_.-]key|secret[_.-]key|" r"client_secret|credentials?|oauth[_-]?token|password|passphrase|" @@ -634,26 +633,6 @@ def is_sensitive_key_name(name: str) -> bool: return _is_sensitive_key(name) -_CAMEL_BOUNDARY_RE = re.compile(r"(?<=[a-z0-9])(?=[A-Z])") -_NAMES_A_CREDENTIAL_RE = re.compile(r"(?i)(?:^|_)(?:" + _CREDENTIAL_KEY_WORDS + r")$") - - -def names_a_credential(name: str) -> bool: - """True when the key ``name`` itself names a credential, as the text - layer's ``key=value`` rule reads one: ``db_password``, ``apiKey``, - ``private-key`` and ``clientSecret`` do; ``maxTokensPerDay``, - ``secretRef``, ``tokenEnv``, ``authz`` and ``author`` do not. - - camelCase, ``-`` and ``.`` read as ``_``, and the name must END with a - credential word. Narrower than :func:`is_sensitive_key_name`, which - matches a sensitive word anywhere in a key: right for masking a logging - payload, too broad where hiding a value costs the reader, such as a - diagnostic that says what a field holds. - """ - snake = _CAMEL_BOUNDARY_RE.sub("_", name).replace("-", "_").replace(".", "_") - return bool(_NAMES_A_CREDENTIAL_RE.search(snake)) - - def redact_value(value: Any) -> Any: """Recursively redact secret-like values in logging payloads.""" if isinstance(value, str): diff --git a/fluid_build/providers/local/local.py b/fluid_build/providers/local/local.py index f1e782da..9b6f9f83 100644 --- a/fluid_build/providers/local/local.py +++ b/fluid_build/providers/local/local.py @@ -416,7 +416,7 @@ def plan( return actions except Exception as e: - self._log_error("local_plan_error", {"error": str(e)}) + self._log_error("local_plan_error", {"error": redact_secret_text(str(e))}) raise def apply( @@ -811,7 +811,12 @@ def _register_with_retry(): except Exception as e: self._log_error( "local_load_data_error", - {"i": idx, "error": str(e), "path": str(path), "table": table_name}, + { + "i": idx, + "error": redact_secret_text(str(e)), + "path": str(path), + "table": table_name, + }, ) raise finally: @@ -880,7 +885,7 @@ def _execute_sql(): "local_sql_error", { "i": idx, - "error": str(e), + "error": redact_secret_text(str(e)), "sql_preview": redacted_sql[:500], "duration_ms": duration_ms(start_time), }, @@ -903,7 +908,8 @@ def _execute_sql(): ) except Exception as e: self._log_warn( - "local_sql_create_table_warn", {"table": output_table, "error": str(e)} + "local_sql_create_table_warn", + {"table": output_table, "error": redact_secret_text(str(e))}, ) written: List[str] = [] @@ -1115,7 +1121,7 @@ def _run_copy_action(self, idx: int, action: Dict[str, Any]) -> Dict[str, Any]: except Exception as e: self._log_warn( "local_materialize_fallback", - {"i": idx, "source_table": source_table, "error": str(e)}, + {"i": idx, "source_table": source_table, "error": redact_secret_text(str(e))}, ) if src and not source_table: diff --git a/fluid_build/providers/local/util/logging.py b/fluid_build/providers/local/util/logging.py index 4871c3e1..e50d70bc 100644 --- a/fluid_build/providers/local/util/logging.py +++ b/fluid_build/providers/local/util/logging.py @@ -91,9 +91,10 @@ def redact_string(text: str) -> str: # The two layers the Snowflake twin runs first: the credentials this # process holds, masked whole (before a pattern below can cut one short), - # then any ``scheme://user:password@`` userinfo. ``_log_error`` passes a # pragma: allowlist secret - # raw ``str(e)`` through here into the record's ``ctx``, which a JSON log - # formatter renders, and the patterns below knew only ``postgresql://``. + # then any ``scheme://user:password@`` userinfo, which the patterns below # pragma: allowlist secret + # knew only for ``postgresql://``. The record's ``ctx`` goes out through + # here, and a JSON log formatter renders it. An exception's text gets the + # full shared redactor where the provider logs it (``local.py``). redacted = redact_known_values(text, placeholder="[REDACTED]") redacted = _URL_USERINFO_RE.sub(r"\1[REDACTED]\2", redacted) diff --git a/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py b/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py index 2b7276ba..ffc149e4 100644 --- a/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py +++ b/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py @@ -155,7 +155,7 @@ def _mission(tmp_path, criteria): " database: sales\n" f" password: '{_SECRET}'\n" f" api_token: '{_SECRET}'\n" - f" settings: {{user: app, password: '{_SECRET}'}}\n", + f" settings: {{user: app, auth: '{_SECRET}'}}\n", encoding="utf-8", ) spec = MissionSpec( @@ -228,9 +228,43 @@ def _diagnostic(path: str, leaf, op="eq", value="x") -> str: @pytest.mark.parametrize( - "path", ["a.privateKey", "a.private-key", "a.apiKey", "a.clientSecret", "a.sessionToken"] + "path", + [ + "a.password", + "a.passwords", + "a.secrets", + "a.secrets.db", + "a.tokens", + "a.auth", + "a.auth.basic", + "a.bearer", + "a.jwt", + "a.secretValue", + "a.private_key_pem", + "a.credentials_json", + "a.credentials.user", + "a.connection_string", + "a.connectionString", + "a.conn_str", + "a.dsn", + "a.PGPASSWORD", + "a.DBPassword", + "a.AWSSecretAccessKey", + "a.MYSQL_PWD", + "a.passwd", + "a.password1", + "a.DB Password", + "a.privateKey", + "a.private-key", + "a.apiKey", + "a.clientSecret", + "a.sessionToken", + ], ) -def test_a_camel_or_kebab_case_credential_key_is_masked(path): +def test_a_credential_key_is_masked(path): + """The key rule fails closed: anything the shared ``is_sensitive_key_name`` + calls a credential (plurals, containers, ``*Value``, no separator) hides + the value, and so does a key under it.""" assert _SECRET not in _diagnostic(path, _SECRET) @@ -241,12 +275,16 @@ def test_a_camel_or_kebab_case_credential_key_is_masked(path): ("policy.authz.readers", ["group:everyone"], "contains", "group:x", "'group:everyone'"), ("binding.secretRef.provider", "vault", "eq", "env", "got 'vault'"), ("sink.catalogAuth.tokenEnv", "UC_TOKEN", "eq", "X", "got 'UC_TOKEN'"), + ("sink.catalogAuth.type", "oauth2", "eq", "bearer", "got 'oauth2'"), + ("sink.catalogAuth.scope", "lakekeeper", "eq", "x", "got 'lakekeeper'"), + ("binding.passwordFile", "/run/pw", "eq", "x", "got '/run/pw'"), ("metadata.author", "alice", "eq", "bob", "got 'alice'"), ], ) -def test_a_key_that_only_mentions_a_credential_word_is_shown(path, leaf, op, value, shown): - """``auth``/``token``/``secret`` inside a longer key name is not a credential: - the diagnostic must keep the value the planner needs to repair it.""" +def test_a_key_that_refers_to_or_only_mentions_a_credential_is_shown(path, leaf, op, value, shown): + """A reference to a credential (``secretRef``, ``tokenEnv``, anything under + ``catalogAuth`` that names one), a token budget and ``authz``/``author`` are + not credentials: the diagnostic keeps the value the planner needs.""" assert shown in _diagnostic(path, leaf, op, value) @@ -289,6 +327,42 @@ def emit(self, record): assert "nohost/x.csv" in repr(records[-1].__dict__) +@pytest.mark.parametrize( + "error", + [ + "HTTP 401 https://api/x?token={s}&api_key={s}", + "auth failed: client_secret={s}", + "Authorization: Bearer {s}", + "aws_secret_access_key={s}", + ], +) +def test_a_failed_sql_action_logs_its_error_redacted(tmp_path, error): + """``_run_sql_on`` logs ``local_sql_error`` with the exception's text; that + text gets the full shared redactor, not only the provider's patterns.""" + from unittest import mock + + from fluid_build.providers.local.local import LocalProvider + + provider = LocalProvider( + logger=logging.getLogger("test.local.sqlerr"), anchor_dir=str(tmp_path) + ) + with mock.patch.object(provider, "_log_error") as log_error, pytest.raises(RuntimeError): + provider._run_sql_on( + _FailingConnection(error.format(s=_SECRET)), 0, {}, "SELECT 1", [], [], 0.0 + ) + assert log_error.call_args_list, "nothing was logged" + logged = repr([c.args for c in log_error.call_args_list]) + assert _SECRET not in logged + + +class _FailingConnection: + def __init__(self, message: str) -> None: + self.message = message + + def sql(self, _sql): + raise RuntimeError(self.message) + + def test_local_redact_sql_masks_a_postgres_url_password(): from fluid_build.providers.local.util.logging import redact_sql @@ -378,6 +452,13 @@ def test_env_prefix_is_iceberg_syntax_for_the_sink_only(): @pytest.mark.parametrize( "key", [ + "database.password", + "mongodb.password", + "mongodb.connection.string", + "client.secret", + "s3.session-token", + "oauth2.credential", + "sasl.jaas.config", "connectionstring", "header.Authorization", "client.authParams", @@ -394,6 +475,19 @@ def test_more_credential_property_names_go_to_the_environment(key): assert list(env.values()) == [_SECRET] +def test_every_catalog_auth_secret_name_is_a_credential_property(): + """The renderer's rule and catalogAuth's own list of secret-valued + properties cannot drift apart.""" + from fluid_build.build_runners.debezium.runner import _CREDENTIAL_PROPERTY_RE + from fluid_build.providers._iceberg_catalog_auth import ( + AUTHORIZATION_HEADER_PROPERTY, + INLINE_CHECKED_KEYS, + ) + + for key in (*INLINE_CHECKED_KEYS, AUTHORIZATION_HEADER_PROPERTY): + assert _CREDENTIAL_PROPERTY_RE.search(key), key + + def test_two_properties_on_one_env_name_fail_closed(): from fluid_build.build_runners.debezium.runner import render_application_properties @@ -406,10 +500,13 @@ def test_embedded_server_file_holds_no_password_and_server_gets_it(tmp_path, mon from fluid_build.build_runners.debezium import runner as dbz monkeypatch.setenv("PG_PASSWORD_T211", _SECRET) + monkeypatch.setenv("LAKEKEEPER_CREDENTIAL_T211", "inherited") captured = tmp_path / "server-env.txt" server = tmp_path / "debezium-server" server.write_text( - f"#!/bin/sh\nprintf '%s' \"$FLUID_DBZ_SOURCE_DATABASE_PASSWORD\" > '{captured}'\nexit 3\n" + "#!/bin/sh\n" + 'printf \'%s|%s\' "$FLUID_DBZ_SOURCE_DATABASE_PASSWORD" "$LAKEKEEPER_CREDENTIAL_T211"' + f" > '{captured}'\nexit 3\n" ) server.chmod(0o700) ctx = SimpleNamespace( @@ -438,7 +535,8 @@ def test_embedded_server_file_holds_no_password_and_server_gets_it(tmp_path, mon config = tmp_path / ".fluid/debezium/sales.orders/b1/application.properties" assert _SECRET not in config.read_text() assert stat.S_IMODE(config.stat().st_mode) == 0o600 - assert captured.read_text() == _SECRET + # the credential is added to the inherited environment, not instead of it + assert captured.read_text() == f"{_SECRET}|inherited" def _embedded_ctx(tmp_path): From e7c32f6d45f9ddfd7b93bb507eddad11205c05f4 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Tue, 13 Oct 2026 23:11:29 +0200 Subject: [PATCH 5/6] fix(mission): a credential URL and a credential nested in a mapping leaf stay masked From the round-2 reviews of 915dc377: - url/uri/endpoint no longer mark a key as a reference: connection_url, credentials.url and secrets.database_url can carry an ODBC/JDBC Pwd= that no text rule recognises (fa8aa5e8 masked them). - a mapping-valued leaf gets the same key rule per entry before redact_value, so pwd, dsn, connectionString and accountKey inside it are masked. - the extra names also cover accountKey, sharedAccessKey, signingKey, encryptionKey, masterKey, storageKey, sshKey, subscriptionKey, passcode and sas. Tests: ordered comparisons on a credential path, nested mapping keys, the invoke_tap timeout path, and a guard that no raw str(e) is logged by the local provider. CHANGELOG: a signal-killed fluid can leave the Singer config in its private temp dir. --- CHANGELOG.md | 3 +- fluid_build/copilot/missions/checks.py | 37 +++++++++++--- ...tials_stay_out_of_files_and_diagnostics.py | 50 ++++++++++++++++++- 3 files changed, 81 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 84390049..b3fadaf3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -124,7 +124,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `env:` value is still written as it is. A hand-written sink key outside that list is written as before, in a file that is now created at mode 0600. - A Meltano / Singer tap and target now read their `--config` file from a private - temporary directory that is removed when the tap or target exits, instead of + temporary directory that is removed when the tap or target exits (a `fluid` process + killed by a signal can leave it there), instead of `.fluid/meltano///tap_config.json` and `target_config.json`. A copy an earlier release left there is deleted on the next run. A tap that writes a refreshed token back to its config file loses that write; the next run resolves the credential diff --git a/fluid_build/copilot/missions/checks.py b/fluid_build/copilot/missions/checks.py index 7b6a7c0d..37ac04b9 100644 --- a/fluid_build/copilot/missions/checks.py +++ b/fluid_build/copilot/missions/checks.py @@ -342,17 +342,22 @@ def _is_number(value: Any) -> bool: #: A key whose last word makes it a REFERENCE to a credential, not one: #: ``secretRef``, ``tokenEnv``, ``credentialEnv``, ``catalogAuth.type``, -#: ``oauth2ServerUri``, ``passwordFile``. +#: ``passwordFile``. ``url``/``uri`` are not here: a ``connection_url`` can +#: carry ``Pwd=`` in a form no text rule recognises. _REFERENCE_WORDS = frozenset( - {"ref", "env", "type", "kind", "mode", "provider", "uri", "url", "endpoint"} - | {"scope", "audience", "resource", "name", "file", "path", "enabled"} + {"ref", "env", "type", "kind", "mode", "provider", "scope", "audience", "resource"} + | {"name", "file", "path", "enabled"} ) #: Words that only contain a sensitive part (``auth``): ``authz``, ``author``. _NOT_CREDENTIAL_WORDS = frozenset({"authz", "authn", "author", "authors", "authority"}) #: A token budget, not a token: ``maxTokensPerDay``, ``tokensPerMinute``. _TOKEN_BUDGET_RE = re.compile(r"(?:^|_)(?:max|min)_tokens(?:_|$)|(?:^|_)tokens_per(?:_|$)") -#: Spellings the shared key rule does not know: ``MYSQL_PWD``, ``passwd``, ``dsn``. -_MORE_CREDENTIAL_NAMES_RE = re.compile(r"(?:passwd|pwd|dsn|connectionstring)$") +#: Spellings the shared key rule does not know: ``MYSQL_PWD``, ``passwd``, +#: ``dsn``, ``accountKey``, ``sharedAccessKey``, ``signingKey``, ``sas``. +_MORE_CREDENTIAL_NAMES_RE = re.compile( + r"(?:passwd|pwd|passcode|dsn|connectionstring|sas|sastoken" + r"|(?:access|account|sharedaccess|signing|encryption|master|storage|ssh|subscription)key)$" +) _KEY_WORD_BREAK_RE = re.compile(r"[^A-Za-z0-9]+|(?<=[a-z0-9])(?=[A-Z])") @@ -406,11 +411,31 @@ def _display_leaf(concrete: str, leaf: Any) -> Any: if not leaf_is_a_reference and any(_key_hides_its_value(key) for key in keys): return _REDACTED try: - return redact_value(leaf) + return redact_value(_mask_credential_keys(leaf)) except RecursionError: return _REDACTED +def _mask_credential_keys(value: Any) -> Any: + """``value`` with every mapping entry under a key that holds a credential + replaced, by the same rule as a path key (``redact_value``'s own key rule + lacks ``pwd``, ``dsn``, ``accountKey`` and the rest).""" + from fluid_build.observability.secret_redactor import _REDACTED + + if isinstance(value, dict): + return { + key: ( + _REDACTED + if isinstance(key, str) and _key_hides_its_value(key) + else _mask_credential_keys(item) + ) + for key, item in value.items() + } + if isinstance(value, list): + return [_mask_credential_keys(item) for item in value] + return value + + def _compare(op: str, leaf: Any, expected: Any, *, shown: Any) -> Tuple[bool, str]: """Evaluate one leaf. Returns ``(ok, reason_if_not)``. Type mismatches fail with a reason rather than raising — fail closed, never crash. diff --git a/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py b/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py index ffc149e4..f848b52a 100644 --- a/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py +++ b/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py @@ -67,8 +67,9 @@ def test_old_name_still_imports_for_library_callers(): def test_no_module_calls_the_old_name(): """CodeQL treats a call to a function named like ``*secret*`` as a source of secrets. A call to ``mask_known_secrets`` made the redactor's OUTPUT a - "secret", and 14 alerts followed it into log lines and files that only - ever held redacted text. Call ``redact_known_values``.""" + "secret", so each log line or file that carried redacted text was reported + as a clear-text leak (14 alerts on main had that source). Call + ``redact_known_values``.""" root = Path(sr.__file__).resolve().parents[1] callers = [] for path in root.rglob("*.py"): @@ -259,6 +260,12 @@ def _diagnostic(path: str, leaf, op="eq", value="x") -> str: "a.apiKey", "a.clientSecret", "a.sessionToken", + "a.connection_url", + "a.credentials.url", + "a.accountKey", + "a.sharedAccessKey", + "a.signingKey", + "a.sas", ], ) def test_a_credential_key_is_masked(path): @@ -288,6 +295,20 @@ def test_a_key_that_refers_to_or_only_mentions_a_credential_is_shown(path, leaf, assert shown in _diagnostic(path, leaf, op, value) +@pytest.mark.parametrize( + "op, value", [("lt", "aaa"), ("lte", "aaa"), ("gt", "zzz"), ("gte", "zzz"), ("contains", "x")] +) +def test_every_comparison_masks_a_credential_leaf(op, value): + assert _SECRET not in _diagnostic("a.password", _SECRET, op, value) + + +@pytest.mark.parametrize("nested", ["pwd", "dsn", "connectionString", "accountKey", "auth"]) +def test_a_credential_key_inside_a_mapping_leaf_is_masked(nested): + diagnostic = _diagnostic("a.settings", {"host": "h", nested: _SECRET}, "eq", {"host": "x"}) + assert _SECRET not in diagnostic + assert "'host': 'h'" in diagnostic + + def test_a_self_referential_leaf_does_not_flip_the_predicate(tmp_path): from fluid_build.copilot.missions.checks import _check_predicate from fluid_build.copilot.missions.spec import CriterionSpec @@ -363,6 +384,16 @@ def sql(self, _sql): raise RuntimeError(self.message) +def test_no_local_provider_log_call_carries_a_raw_exception_text(): + """Every ``"error": str(e)`` the local provider logs goes through the shared + redactor (``local.py`` logs a plan, load, SQL, create-table and copy error).""" + import fluid_build.providers.local.local as local_module + + source = Path(local_module.__file__).read_text(encoding="utf-8") + assert '"error": str(e)' not in source + assert source.count("redact_secret_text(str(e))") >= 6 + + def test_local_redact_sql_masks_a_postgres_url_password(): from fluid_build.providers.local.util.logging import redact_sql @@ -699,6 +730,21 @@ def test_invoke_target_config_is_private_and_removed(tmp_path): assert not stale.exists() +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX file modes and a script tap") +def test_invoke_tap_removes_the_config_after_a_timeout(tmp_path): + from fluid_build.build_runners.meltano.runner import invoke_tap + + with pytest.raises(subprocess.TimeoutExpired): + invoke_tap( + str(_fake_tap(tmp_path, 0, sleep=120)), + config=_tap_config(), + workdir=tmp_path / "w", + timeout_seconds=8, + ) + seen = json.loads((tmp_path / "tap-saw.json").read_text()) + assert not Path(seen["path"]).exists() + + # ── fluid init: generated connector config is git-ignored ──────────────── From 6aa7a3b7ae35d47f7705061fba85da18cec9b6b9 Mon Sep 17 00:00:00 2001 From: Speculator55005 <50082482+fas89@users.noreply.github.com> Date: Tue, 13 Oct 2026 23:28:45 +0200 Subject: [PATCH 6/6] test: one import style for fluid_build.providers.local.local in the credential tests --- .../test_credentials_stay_out_of_files_and_diagnostics.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py b/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py index f848b52a..c1b4a913 100644 --- a/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py +++ b/tests/security/test_credentials_stay_out_of_files_and_diagnostics.py @@ -387,9 +387,11 @@ def sql(self, _sql): def test_no_local_provider_log_call_carries_a_raw_exception_text(): """Every ``"error": str(e)`` the local provider logs goes through the shared redactor (``local.py`` logs a plan, load, SQL, create-table and copy error).""" - import fluid_build.providers.local.local as local_module + import inspect - source = Path(local_module.__file__).read_text(encoding="utf-8") + from fluid_build.providers.local.local import LocalProvider + + source = Path(inspect.getsourcefile(LocalProvider) or "").read_text(encoding="utf-8") assert '"error": str(e)' not in source assert source.count("redact_secret_text(str(e))") >= 6