[PR] Document Connection-extras allowlist rule for provider hooks/operators [airflow]

["potiuk (via GitHub)" <gi...@apache.org>]

potiuk opened a new pull request, #66032:
URL: https://github.com/apache/airflow/pull/66032

   Adds a "Security: Connection extras must not be forwarded blindly to hooks/operators" section to `providers/AGENTS.md`. The section spells out:
   
   - The trust boundary — Connection editors and DAG authors are different populations with different privileges.
   - Concrete failure modes when extras are spread blindly into client libraries (RCE, file read/write, MITM, auth bypass).
   - The rule: allowlist named keys, never `**conn.extra_dejson` passthrough.
   - Patterns to flag in provider PR review.
   
   This is a docs-only change to give agents (and human reviewers) a clear rule when writing or reviewing provider hooks and operators.
   
   ---
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes — Claude Opus 4.7 (1M context)
   
   Generated-by: Claude Opus 4.7 (1M context) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Document Connection-extras allowlist rule for provider hooks/operators [airflow]

["potiuk (via GitHub)" <gi...@apache.org>]

potiuk merged PR #66032:
URL: https://github.com/apache/airflow/pull/66032


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Document Connection-extras allowlist rule for provider hooks/operators [airflow]

["shahar1 (via GitHub)" <gi...@apache.org>]

shahar1 commented on code in PR #66032:
URL: https://github.com/apache/airflow/pull/66032#discussion_r3155963633


##########
providers/AGENTS.md:
##########
@@ -17,3 +17,77 @@ Each provider is an independent package with its own `pyproject.toml`, tests, an
 - Don't upper-bound dependencies by default; add limits only with justification.
 - Tests live alongside the provider — mirror source paths in test directories.
 - Full guide: [`contributing-docs/12_provider_distributions.rst`](../contributing-docs/12_provider_distributions.rst)
+
+## 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

Review Comment:
   nit:
   ```suggestion
     option should be a **Dag-author-supplied argument** on the operator or
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org