github-actions[bot] opened a new pull request, #67173:
URL: https://github.com/apache/airflow/pull/67173
* Refuse secrets-backend fallback on Execution-API authz deny
ExecutionAPISecretsBackend.get_connection / get_variable returned None
on every ErrorResponse, conflating "not found" with "explicitly denied".
The secrets-backend dispatcher then fell through to the next backend
(typically EnvironmentVariablesBackend, which performs no authz checks)
on a 401/403 from the Execution API -- letting tasks read secrets the
Execution API had just denied them. The audit calls this a Type C gap:
the authz control fires, but its rejection result is treated as a
miss and routed around.
Three-part fix:
1. New `ErrorType.PERMISSION_DENIED` distinct from `API_SERVER_ERROR`.
2. `ConnectionOperations.get` and `VariableOperations.get` map the
API server's 401/403 to `ErrorResponse(PERMISSION_DENIED, ...)`
instead of re-raising as a generic `ServerResponseError`. 404 still
maps to `*_NOT_FOUND`; other statuses still raise so the existing
API_SERVER_ERROR translation in `handle_requests` keeps working.
3. `ExecutionAPISecretsBackend` (sync + async, connection + variable
variants) now raises `PermissionError` on `PERMISSION_DENIED`. The
surrounding `except Exception:` blocks explicitly re-raise
`PermissionError` so the secrets-backend dispatcher sees it. NOT_FOUND
types continue to return `None` (allow fallthrough); other
ErrorResponses also continue to return `None` (preserve existing
recovery behaviour for transient errors).
Tests added:
- `client.connections.get` and `client.variables.get` return
`ErrorResponse(PERMISSION_DENIED)` on 401 and 403 (parametrised).
- `ExecutionAPISecretsBackend.get_connection` / `get_variable` /
`aget_connection` / `aget_variable` raise `PermissionError` when the
response is `PERMISSION_DENIED`, with the resource and key in the
message.
Reported by the L3 ASVS sweep at apache/tooling-agents#24 (FINDING-017).
* Refuse secrets-backend fallback at the dispatcher, not only the backend
The earlier change made ExecutionAPISecretsBackend raise on 401/403, but
the dispatcher loops in airflow.sdk.execution_time.context and the
airflow-core get_*_from_secrets paths catch Exception and silently fall
through to the next backend — so the deny was still being swallowed.
Introduce AirflowSecretsBackendAccessDenied (subclass of PermissionError)
so the dispatchers can special-case the authoritative deny without
mis-treating an incidental OSError-family PermissionError from inside an
unrelated backend. Patch the three task-SDK dispatcher loops and the two
airflow-core dispatcher loops to re-raise it before the generic except.
Add TestDispatcherRefusesFallbackOnDeny with three end-to-end tests that
insert a spy backend after ExecutionAPISecretsBackend and assert the spy
is never called once the first backend raises the deny — pinning the
dispatcher behaviour, not just the backend's. Also hoist the repeated
imports in test_secrets.py to module top per review feedback.
* Hoist AirflowSecretsBackendAccessDenied imports to module top
(cherry picked from commit 2b8c80568b5dfaea54f367ab08c33d943a80818a)
Co-authored-by: Jarek Potiuk <[email protected]>
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
