This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new c8e0ec564a7 Document Connection-extras allowlist rule for provider
hooks/operators (#66032)
c8e0ec564a7 is described below
commit c8e0ec564a7a4633533a50bc0f77c7f35f534f58
Author: Jarek Potiuk <[email protected]>
AuthorDate: Thu Apr 30 20:07:10 2026 +0200
Document Connection-extras allowlist rule for provider hooks/operators
(#66032)
* Document that providers must not forward Connection extras blindly
Add a security section to providers/AGENTS.md explaining the trust
boundary between Connection editors and DAG authors, and the
allowlist rule for forwarding Connection.extra values to hooks,
operators, and client libraries.
* Update providers/AGENTS.md
Co-authored-by: Shahar Epstein <[email protected]>
---------
Co-authored-by: Shahar Epstein <[email protected]>
---
providers/AGENTS.md | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 74 insertions(+)
diff --git a/providers/AGENTS.md b/providers/AGENTS.md
index 254c8df7436..bf75c002d4e 100644
--- a/providers/AGENTS.md
+++ b/providers/AGENTS.md
@@ -31,3 +31,77 @@ change that warrants explanation), update the provider's
`docs/changelog.rst` di
PR, just below the `Changelog` header — exactly as the in-file `NOTE TO
CONTRIBUTORS` block
describes. Routine entries (features, bug fixes, misc) are collected
automatically by the release
manager from commit messages, so most PRs do not need to touch the changelog
at all.
+
+## Security: Connection extras must not be forwarded blindly to hooks/operators
+
+**Never pass the whole `Connection.extra` dict (or `**conn.extra_dejson`) as
+keyword arguments to a hook, operator, client constructor, or any underlying
+library call.** Forward only the specific extra keys you have explicitly
+reviewed and know are safe to expose.
+
+### Why this matters — different security boundaries
+
+Airflow has two distinct user roles with different trust levels:
+
+- **Connection editors** — UI/API users with permission to create or edit
+ Connections. They control the host, login, password, and the `extra` JSON
+ blob.
+- **DAG authors** — users who write the Python code that constructs hooks
+ and operators. They control which arguments are passed at call sites.
+
+These are *not* the same population, and the security model treats them
+differently. A Connection editor is trusted to supply credentials for a target
+system; they are **not** trusted to alter how the worker process behaves, load
+arbitrary Python code, change file paths the worker reads, or pass options
+into client libraries that the DAG author did not opt into.
+
+### What goes wrong when extras are forwarded blindly
+
+Many client libraries accept constructor or method kwargs that are dangerous
+when attacker-controlled. Concrete examples seen in the wild:
+
+- A kwarg that names a Python callable, plugin path, or import string —
+ attacker sets it to a module they control → **remote code execution** on
+ the worker.
+- A kwarg that points at a local file (cert path, key file, log path,
+ config file) — attacker redirects it to read or overwrite worker-side
+ files.
+- A kwarg that sets a proxy, endpoint URL, or hostname — attacker
+ redirects traffic to an MITM endpoint and harvests credentials or
+ tokens for *other* systems.
+- A kwarg that toggles TLS verification, signing, or auth — attacker
+ silently downgrades security.
+- A kwarg that controls subprocess execution, shell invocation, or
+ template rendering — attacker reaches command/template injection.
+
+In all of these, the Connection editor effectively gains capabilities
+(RCE, file read/write, traffic redirection, auth bypass) that the security
+model does not grant them.
+
+### The rule
+
+- **Allowlist, never passthrough.** In the hook, read each extra key by
+ name (`conn.extra_dejson.get("region_name")`,
+ `conn.extra_dejson.get("verify")`, …) and pass only those named values
+ forward. Reject or ignore unknown keys.
+- **Do not** write `SomeClient(**conn.extra_dejson)`,
+ `hook = MyHook(**conn.extra_dejson)`, or
+ `kwargs.update(conn.extra_dejson)` followed by a downstream call.
+- When adding support for a new extra key, treat it like any other public
+ argument: review what the underlying library does with it, and document
+ it in the provider's connection docs.
+- If a DAG author genuinely needs to pass a non-allowlisted option, that
+ option should be a **Dag-author-supplied argument** on the operator or
+ hook (with its own review), not something a Connection editor can set.
+
+### When reviewing provider PRs
+
+Flag any of these patterns:
+
+- `**conn.extra_dejson` or `**self.extra_dejson` spread into a constructor
+ or call.
+- Looping over `conn.extra_dejson.items()` and forwarding every key.
+- New code paths where extras are merged into `kwargs` and then passed on
+ unfiltered.
+- "Convenience" features that let users put arbitrary client kwargs into
+ `extra` — they widen the Connection-editor blast radius.
