[fas89]

Second and final PR for the roadmap board's "[P1][Security][forge-cli] Acquisition secretRef env:// sends any environment variable to a host the contract names" (Trello 1025). The first half shipped as #750.

The defect

connection.secretRef: env://NAME let the contract choose both halves — the environment 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 to a host the contract picked.

All six acquisition runners go through resolve_connection_secrets: airbyte, debezium, dlt, duckdb, kafka_connect, meltano. The card named four — debezium and kafka_connect are callers too.

The policy

Option B, as decided: env:// 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.

• It lives in resolve_connection_secrets, the one place that sees both halves, so all six runners inherit it without any runner changing.
• It runs before the reference resolves, so a refused send never reads the secret, and the refusal names the variable and host but never the value — asserted over the whole rendered error, not one field.
• Only env:// is policed. vault://, aws://, gcp://, azure:// and file:// name a secret the operator provisioned and granted this process access to, which is already an operator decision.
• The allowlist is operator-only via credentials/protected_env.py, because fluid apply hydrates .env, .env.<env> and FLUID_SECRETS_FILE, which are committed beside the contract. Without that, a contract repository could grant itself the allowlist meant to constrain it. The existing federation mechanism was reused, not reimplemented.

Two things the work itself turned up

• TESTCONTAINERS_HOST_OVERRIDE was not operator-only while FLUID_RUNNER_HOST_OVERRIDE was. Both rewrite a contract-declared loopback host to a reachable one after the policy clears the send as local-only, so a project .env able to set either could have redirected a credential allowed only because it was staying on this machine — defeating the single allowance this policy grants. Now registered.
• I had written a second loopback check. _acquisition_common._is_loopback_host already existed and is what apply_loopback_host_override rewrites on; the exemption is sound only while the two cover identical hosts, since a wider one would exempt a send the override leaves pointing at a real host. It now delegates to that one definition, with a test asserting they agree over nine cases.

What the adversarial review found

A four-lens review of this branch returned bypass=FAIL, correctness=FAIL, 8 must-fix. All are fixed in 87545ca5; three were my own mistakes and are worth stating plainly:

• A working bypass. The policy trusted the first destination field that parsed, so a contract could set host: localhost as a decoy beside a remote connection_string (debezium's mongodb shape) and have the send cleared on the decoy while the credential went to the remote host. Every destination is now collected and all must clear; the field set gained connection_string, server and endpoint.
• A false claim in an earlier commit message. It said the airbyte deployment.auth.secretRef path was policed. It was not — that edit had failed on a bad anchor, I noted "nothing written" at the time, and then asserted it was done. It is policed now, and a test reads the module for the call site rather than trusting the claim.
• Fail-open parsing. An allowlist entry NAME@ — a name with an empty host — granted every host, and my own "malformed entry grants nothing" test did not cover it. It now grants nothing, as federation does.

Plus, each with a test: an inline credential is no longer policed (the reference is never read, which the upgrade note already promised); an object-store URI is no longer read as a host, since its authority is a bucket; localhost. is loopback; AcquisitionSecretPolicyError was missing @dataclass so its code reported FluidUserError; the duckdb runner no longer flattens the refusal through except Exception and drops the fix; and the fix now warns that a host written with a port never matches, which is the trip-up federation already warns about.

The limitation that is documented, not closed

The review's sharpest point was not a code defect. The earlier CHANGELOG asserted "the set of contracts this breaks is exactly the set that was vulnerable" and that loopback "needs no configuration and is unaffected". Both are false for kafka_connect and the default bring-your-own / managed modes of debezium and airbyte: there forge-cli never connects to the database at all — it hands the whole connector config, resolved credential included, to the orchestrator at a contract-chosen properties.<engine>.deployment.server_url, which nothing allowlists. A loopback connection.host on those paths is a zero-configuration exfiltration, and it needs no allowlist entry, so it is arguably easier than the route this PR blocks.

Verified unchanged from before this work (resolve_connection_secrets on upstream/main has no policy call, and KafkaConnectRestClient is byte-identical), so nothing regressed — but publishing a guarantee that does not hold is worse than publishing none. There is now a ### Known limitations section naming it, alongside the object-store gap and {{ env.NAME }} interpolation, which bypasses secretRef entirely. 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; that gap wants its own card.

Validation

• Full suite, CI-shaped, both versions: 4 failures on duckdb 2.0.0 and 1.5.6 — the same four that fail identically on clean main. 0 caused by this change.
• Eight existing tests refused correctly and now configure the allowlist, which is the one-line change an operator makes on upgrade, rather than the policy being loosened to accommodate them. The eighth was only caught by the full suite, in the debezium credential-hygiene suite.
• 50 tests in the policy file; one per review finding.
• ruff and CI-pinned black==24.10.0 clean whole-tree; lint-imports 6 contracts kept, 0 broken; startup budget 6/6; detect-secrets hook clean over the changed files; duckdb-floor gate 3/3.
• ripwire --quality-delta gating 16, exactly the base — no quality regressions.

Documentation

• No docs needed — this change does not affect user-facing behaviour

It does change behaviour for a contract using env:// against a remote host, and that is covered in CHANGELOG.md under ### Upgrade notes with the exact variable to set, the warning that a project .env is ignored, and the note that a host written with a port never matches.