security: a contract could send any environment variable to a host it named

CHANGELOG.md

@@ -26,6 +26,51 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   declared DSN still carries its own credentials, so a legitimate source is unaffected and no
   contract change is needed.
 
+- **A contract could send any environment variable to a host of its own choosing.**
+  `connection.secretRef: env://NAME` let the contract name both halves — the variable to read and
+  the host the resolved value is sent to as a password — and nothing checked either. A run whose
+  environment holds secrets (CI, an operator's shell) could therefore have any variable
+  exfiltrated. All six acquisition runners (airbyte, debezium, dlt, duckdb, kafka_connect,
+  meltano) go through one helper, so all six were affected.
+  `env://` now resolves only when **every** destination the connection block carries is loopback,
+  or when the operator has listed the variable in `FLUID_ACQUISITION_SECRET_REF_ALLOWLIST` as
+  `NAME` (any host) or `NAME@host`. Every destination is checked, not just the first field that
+  parses, so a decoy `host: localhost` beside a remote `connection_string` does not clear the
+  send. The check runs **before** the reference is resolved, so a refused send never reads the
+  secret, and the refusal names the variable and host but never the value. `vault://`, `aws://`,
+  `gcp://`, `azure://` and `file://` are not policed: those name a secret the operator provisioned
+  and granted this process access to, which is already an operator decision. A `secretRef` whose
+  target field already holds an inline credential is not policed either — the reference is never
+  read and nothing is sent.
+  The allowlist is an **operator** setting: a value set by one of the project's own dotenv files is
+  ignored, because `fluid apply` hydrates the environment from `.env`, `.env.<env>` and
+  `FLUID_SECRETS_FILE`, and those files are committed beside the contract.
+- **`TESTCONTAINERS_HOST_OVERRIDE` is now an operator-only setting**, as
+  `FLUID_RUNNER_HOST_OVERRIDE` already was. Both rewrite a contract-declared loopback host to a
+  reachable one *after* the policy above has cleared the send as local-only, so a project dotenv
+  file able to set either could have redirected a credential allowed only because it was staying
+  on this machine.
+- Resolved `secretRef` values are registered with the exact-value redactor (#884), so a literal a
+  client library echoes in its own error is masked by value rather than by pattern.
+
+
+- A contract repository could hand the whole host to its own SQL through DuckDB's spill
+  directory. `secure_duckdb_connect` never set `temp_directory`, and DuckDB resolves its
+  temp directory into `allowed_directories` like any other grant; the default is relative
+  (`.tmp` in the working directory, or `<db>.tmp` beside a file database), so a repository
+  shipping `.tmp -> /` got `allowed_directories = ['/']` on a connection that granted
+  nothing, and contract SQL could read any file the process could. Reproduced on duckdb
+  1.5.1, 1.5.6 and 2.0.0, for an in-memory and a file database, through the same helper the
+  MCP output port's `query_sql` runs on. The engine now creates and owns that directory
+  (mode 0700, under `$TMPDIR`, removed with the connection), reads the grants back under
+  the lock and refuses the connection if DuckDB took a directory the call site did not
+  name, and refuses a caller-supplied `temp_directory` rather than silently overriding it.
+  Spilling is unaffected. No contract, flag or configuration change is needed.
+- Opening one DuckDB database file from two threads at once is now serialised. DuckDB
+  shares one instance per file per process and the sandbox's settings live on the instance,
+  so the two opens could cross-configure: a connection could be returned running under the
+  other thread's allowlist. The second opener now reliably gets the existing
+  "already open in this process" refusal.
 
 ### Fixed
 
@@ -74,27 +119,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   with an invalid type stay invalid — the fix is emit-time — but those already failed
   validation, so nothing working changes.
 
-### Security
+### Known limitations
 
-- A contract repository could hand the whole host to its own SQL through DuckDB's spill
-  directory. `secure_duckdb_connect` never set `temp_directory`, and DuckDB resolves its
-  temp directory into `allowed_directories` like any other grant; the default is relative
-  (`.tmp` in the working directory, or `<db>.tmp` beside a file database), so a repository
-  shipping `.tmp -> /` got `allowed_directories = ['/']` on a connection that granted
-  nothing, and contract SQL could read any file the process could. Reproduced on duckdb
-  1.5.1, 1.5.6 and 2.0.0, for an in-memory and a file database, through the same helper the
-  MCP output port's `query_sql` runs on. The engine now creates and owns that directory
-  (mode 0700, under `$TMPDIR`, removed with the connection), reads the grants back under
-  the lock and refuses the connection if DuckDB took a directory the call site did not
-  name, and refuses a caller-supplied `temp_directory` rather than silently overriding it.
-  Spilling is unaffected. No contract, flag or configuration change is needed.
-- Opening one DuckDB database file from two threads at once is now serialised. DuckDB
-  shares one instance per file per process and the sandbox's settings live on the instance,
-  so the two opens could cross-configure: a connection could be returned running under the
-  other thread's allowlist. The second opener now reliably gets the existing
-  "already open in this process" refusal.
+- **The loopback exemption covers only the engines where this process makes the connection** —
+  dlt, duckdb, meltano, and the `embedded` modes of debezium and airbyte. For kafka_connect and
+  the `bring-your-own` / `managed` modes of debezium and airbyte, forge-cli does not connect to
+  the database at all: it hands the whole connector config, resolved credential included, to the
+  orchestrator named by `properties.<engine>.deployment.server_url`. That URL is contract-chosen
+  and **this policy does not check it**, so a loopback `connection.host` still clears a send whose
+  credential then leaves the machine in an HTTP body. The behaviour is unchanged from before this
+  release — nothing regressed — but it is NOT closed, and `deployment.server_url` has no host
+  allowlist of the kind `FLUID_COMMAND_CENTER_HOST_ALLOWLIST`, `FLUID_FEDERATION_HOST_ALLOWLIST`
+  and `FLUID_WEBHOOK_HOST_ALLOWLIST` already provide elsewhere. Treat an `env://` secretRef on
+  those three paths as unprotected whatever the declared host.
+- A connection whose only destination field is an object-store URI (`s3://`, `gs://`, `az://`,
+  `file://`) is not policed: the authority there is a bucket, not a host, so a host-based policy
+  has nothing to decide.
+- `{{ env.NAME }}` interpolation in a connection field reads the environment without going through
+  `secretRef` at all, so it is not covered by this policy.
 
-### Known limitations
 
 - On DuckDB 1.5.x, `lock_configuration` does not hold for logging reached as a table
   function: `SET enable_logging = true` is refused, but `SELECT * FROM enable_logging(...)`
@@ -104,6 +147,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   output port's `query_sql` under `policy.allow_free_form_sql`. DuckDB 2.0 refuses it
   cleanly.
 
+### Upgrade notes
+
+- **A contract using `env://` against a remote host now fails until the operator allows it.** Each
+  one is restored by setting a single operator-environment variable,
+  `FLUID_ACQUISITION_SECRET_REF_ALLOWLIST`, to the variable's own name — for example
+  `PG_PASSWORD@db.example.com` for that host only, or `PG_PASSWORD` for any host. Set it in the
+  shell, in CI, or in a `FLUID_SECRETS_FILE` outside the project — **not** in a project `.env`,
+  where it is ignored.
+  A host written with a port never matches, so leave the port off. Local development against
+  `localhost`, `127.0.0.0/8` or `::1` needs no configuration. The better long-term answer for a
+  production credential is a secret store: `vault://`, `aws://`, `gcp://`, `azure://` or `file://`.
+- `fluid init --discover` against a remote host emits a contract carrying `env://`, which `fluid
+  apply` will now refuse until that variable is allowlisted. The discover hint does not yet print
+  the allowlist line; the refusal from `apply` does.
+
 ## [0.22.0] — 2026-10-22
 
 ### Upgrade notes

fluid_build/build_runners/_acquisition_common.py

@@ -40,6 +40,11 @@
 from pathlib import Path
 from typing import Any, Callable, Dict, List, Optional, Sequence, Tuple
 
+from fluid_build.build_runners._secret_policy import (
+    refuse_unless_allowed,
+    register_resolved,
+)
+
 LOG = logging.getLogger("fluid.build_runners.acquisition")
 
 # Strips ANSI CSI sequences and ASCII control chars from error strings
@@ -525,7 +530,27 @@ def resolve_connection_secrets(
     secret_ref = out.pop("secretRef", None)
     if not secret_ref:
         return out
+    # The policy runs BEFORE the reference is resolved, so a refused send never
+    # reads the secret. This is the one place that sees both halves the contract
+    # chose -- the variable and the destination -- which is why the check lives
+    # here and all six acquisition runners inherit it.
+    if out.get(target_field):
+        # An inline literal already wins (below), so the reference is never
+        # read and nothing is sent anywhere: there is nothing to police. The
+        # upgrade note promises exactly this, and checking here would refuse a
+        # contract that has a working credential and a stale secretRef beside
+        # it.
+        return out
+    scheme = str(secret_ref).partition("://")[0].strip().lower()
+    if scheme == "env":
+        variable = str(secret_ref).partition("://")[2].strip()
+        # The whole connection, so EVERY destination it carries has to clear.
+        refuse_unless_allowed(variable, connection=out, scheme="env")
     value = resolve_secret_ref(secret_ref)
+    # #884: the literal can be echoed by a client library's own error, which the
+    # runners print. Registering it means the redactor masks it by value rather
+    # than hoping a pattern recognises its shape.
+    register_resolved(value)
     if not out.get(target_field):
         out[target_field] = value
     return out