Refuse secrets-backend fallback on Execution-API authz deny - #66575

Merged
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error
May 19, 2026
Merged

Refuse secrets-backend fallback on Execution-API authz deny#66575
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error

Conversation

@potiuk

@potiukpotiuk commented May 7, 2026

Copy link
Copy Markdown
Member

Summary

The Execution API's authorization decision was being routed around by the
secrets-backend dispatcher: ExecutionAPISecretsBackend.get_connection /
get_variable returned None on every ErrorResponse, conflating "not
found" with "explicitly denied". And even with the backend patched to raise
on a 401/403, the dispatcher loops in
airflow/sdk/execution_time/context.py, airflow/models/connection.py,
and airflow/models/variable.py catch Exception and continue to the
next backend — so the deny would still have been swallowed silently.

Scope of the gap

In the default worker chain
([EnvironmentVariablesBackend, ExecutionAPISecretsBackend]),
ExecutionAPISecretsBackend is last — there is no later backend for the
dispatcher to fall through to, and the gap does not manifest. The
fall-through-leak scenario requires a non-default configuration: a
reordering, or a custom [secrets] backend placed after
ExecutionAPISecretsBackend. Even there, silently downgrading an
authoritative deny to "next backend, please" is wrong on principle: the
audit calls this a Type C gap — the authz control fires, but its
rejection is treated as a miss and routed around.

Fix (four parts)

1. New ErrorType.PERMISSION_DENIED

Distinct from API_SERVER_ERROR so callers can dispatch on the cause.

2. Client maps 401/403 to PERMISSION_DENIED

ConnectionOperations.get and VariableOperations.get translate 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. New AirflowSecretsBackendAccessDenied(PermissionError)

Distinct, dispatcher-aware exception class. Subclasses PermissionError
so any existing except PermissionError: handler still matches, but is
narrow enough that the dispatcher can re-raise only this signal
without accidentally promoting an incidental filesystem
OSError-family PermissionError from inside an unrelated backend.

ExecutionAPISecretsBackend (sync + async, connection + variable
variants) raises this on PERMISSION_DENIED. Other ErrorResponse
types (*_NOT_FOUND, transient API_SERVER_ERROR, GENERIC_ERROR)
continue to return None so existing recovery paths keep working.

4. Dispatchers honour the deny

The three task-SDK dispatcher loops in
airflow/sdk/execution_time/context.py (_get_connection,
_async_get_connection, _get_variable) and the two airflow-core
dispatcher loops in airflow/models/connection.py and
airflow/models/variable.py now catch
AirflowSecretsBackendAccessDenied and re-raise it before the
generic except Exception: fall-through.

Tests

  • client.connections.get / client.variables.get return
    ErrorResponse(PERMISSION_DENIED) on 401 and 403 (parametrised).
  • ExecutionAPISecretsBackend.get_connection / get_variable /
    aget_connection / aget_variable raise
    AirflowSecretsBackendAccessDenied when the response is
    PERMISSION_DENIED.
  • End-to-end dispatcher tests in
    TestDispatcherRefusesFallbackOnDeny insert a spy backend AFTER
    ExecutionAPISecretsBackend and assert it is never called once
    the first backend raises the deny — pinning the dispatcher's
    re-raise behaviour, not just the backend's. Covers
    _get_connection, _get_variable, and _async_get_connection.

Reported by

L3 ASVS sweep — apache/tooling-agents#24 (FINDING-017).


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.7)

Generated-by: Claude Code (Opus 4.7) following the guidelines

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).
@potiuk
potiukforce-pushed the fix/tasksdk-secrets-distinguish-authz-error branch from e522ad9 to 5063192CompareMay 17, 2026 19:30
@potiuk

Copy link
Copy Markdown
MemberAuthor

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)


Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

@potiukpotiuk added this to the Airflow 3.2.2 milestone May 17, 2026
@vatsrahul1001vatsrahul1001 added the ready for maintainer review Set after triaging when all criteria pass. label May 18, 2026
@vatsrahul1001

Copy link
Copy Markdown
Contributor

LGTM! ready for maintainer review

Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py Outdated
Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py
@kaxil

Copy link
Copy Markdown
Member

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)

Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

As-is, it isn't ready, check the comments above

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.
@potiuk
potiuk requested a review from XD-DENG as a code ownerMay 19, 2026 08:18
@potiukpotiuk removed the ready for maintainer review Set after triaging when all criteria pass. label May 19, 2026
@potiuk
potiuk merged commit 2b8c805 into apache:mainMay 19, 2026
113 checks passed
@potiuk
potiuk deleted the fix/tasksdk-secrets-distinguish-authz-error branch May 19, 2026 11:24
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-2-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

StatusBranchResult
v3-2-testPR Link

vatsrahul1001 pushed a commit that referenced this pull request May 19, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 21, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@potiuk@vatsrahul1001@kaxil
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Refuse secrets-backend fallback on Execution-API authz deny - #66575

Merged
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error
May 19, 2026
Merged

Refuse secrets-backend fallback on Execution-API authz deny#66575
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error

Conversation

@potiuk

@potiukpotiuk commented May 7, 2026

Copy link
Copy Markdown
Member

Summary

The Execution API's authorization decision was being routed around by the
secrets-backend dispatcher: ExecutionAPISecretsBackend.get_connection /
get_variable returned None on every ErrorResponse, conflating "not
found" with "explicitly denied". And even with the backend patched to raise
on a 401/403, the dispatcher loops in
airflow/sdk/execution_time/context.py, airflow/models/connection.py,
and airflow/models/variable.py catch Exception and continue to the
next backend — so the deny would still have been swallowed silently.

Scope of the gap

In the default worker chain
([EnvironmentVariablesBackend, ExecutionAPISecretsBackend]),
ExecutionAPISecretsBackend is last — there is no later backend for the
dispatcher to fall through to, and the gap does not manifest. The
fall-through-leak scenario requires a non-default configuration: a
reordering, or a custom [secrets] backend placed after
ExecutionAPISecretsBackend. Even there, silently downgrading an
authoritative deny to "next backend, please" is wrong on principle: the
audit calls this a Type C gap — the authz control fires, but its
rejection is treated as a miss and routed around.

Fix (four parts)

1. New ErrorType.PERMISSION_DENIED

Distinct from API_SERVER_ERROR so callers can dispatch on the cause.

2. Client maps 401/403 to PERMISSION_DENIED

ConnectionOperations.get and VariableOperations.get translate 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. New AirflowSecretsBackendAccessDenied(PermissionError)

Distinct, dispatcher-aware exception class. Subclasses PermissionError
so any existing except PermissionError: handler still matches, but is
narrow enough that the dispatcher can re-raise only this signal
without accidentally promoting an incidental filesystem
OSError-family PermissionError from inside an unrelated backend.

ExecutionAPISecretsBackend (sync + async, connection + variable
variants) raises this on PERMISSION_DENIED. Other ErrorResponse
types (*_NOT_FOUND, transient API_SERVER_ERROR, GENERIC_ERROR)
continue to return None so existing recovery paths keep working.

4. Dispatchers honour the deny

The three task-SDK dispatcher loops in
airflow/sdk/execution_time/context.py (_get_connection,
_async_get_connection, _get_variable) and the two airflow-core
dispatcher loops in airflow/models/connection.py and
airflow/models/variable.py now catch
AirflowSecretsBackendAccessDenied and re-raise it before the
generic except Exception: fall-through.

Tests

  • client.connections.get / client.variables.get return
    ErrorResponse(PERMISSION_DENIED) on 401 and 403 (parametrised).
  • ExecutionAPISecretsBackend.get_connection / get_variable /
    aget_connection / aget_variable raise
    AirflowSecretsBackendAccessDenied when the response is
    PERMISSION_DENIED.
  • End-to-end dispatcher tests in
    TestDispatcherRefusesFallbackOnDeny insert a spy backend AFTER
    ExecutionAPISecretsBackend and assert it is never called once
    the first backend raises the deny — pinning the dispatcher's
    re-raise behaviour, not just the backend's. Covers
    _get_connection, _get_variable, and _async_get_connection.

Reported by

L3 ASVS sweep — apache/tooling-agents#24 (FINDING-017).


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.7)

Generated-by: Claude Code (Opus 4.7) following the guidelines

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).
@potiuk
potiukforce-pushed the fix/tasksdk-secrets-distinguish-authz-error branch from e522ad9 to 5063192CompareMay 17, 2026 19:30
@potiuk

Copy link
Copy Markdown
MemberAuthor

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)


Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

@potiukpotiuk added this to the Airflow 3.2.2 milestone May 17, 2026
@vatsrahul1001vatsrahul1001 added the ready for maintainer review Set after triaging when all criteria pass. label May 18, 2026
@vatsrahul1001

Copy link
Copy Markdown
Contributor

LGTM! ready for maintainer review

Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py Outdated
Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py
@kaxil

Copy link
Copy Markdown
Member

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)

Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

As-is, it isn't ready, check the comments above

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.
@potiuk
potiuk requested a review from XD-DENG as a code ownerMay 19, 2026 08:18
@potiukpotiuk removed the ready for maintainer review Set after triaging when all criteria pass. label May 19, 2026
@potiuk
potiuk merged commit 2b8c805 into apache:mainMay 19, 2026
113 checks passed
@potiuk
potiuk deleted the fix/tasksdk-secrets-distinguish-authz-error branch May 19, 2026 11:24
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-2-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

StatusBranchResult
v3-2-testPR Link

vatsrahul1001 pushed a commit that referenced this pull request May 19, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 21, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@potiuk@vatsrahul1001@kaxil
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Refuse secrets-backend fallback on Execution-API authz deny - #66575

Merged
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error
May 19, 2026
Merged

Refuse secrets-backend fallback on Execution-API authz deny#66575
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error

Conversation

@potiuk

@potiukpotiuk commented May 7, 2026

Copy link
Copy Markdown
Member

Summary

The Execution API's authorization decision was being routed around by the
secrets-backend dispatcher: ExecutionAPISecretsBackend.get_connection /
get_variable returned None on every ErrorResponse, conflating "not
found" with "explicitly denied". And even with the backend patched to raise
on a 401/403, the dispatcher loops in
airflow/sdk/execution_time/context.py, airflow/models/connection.py,
and airflow/models/variable.py catch Exception and continue to the
next backend — so the deny would still have been swallowed silently.

Scope of the gap

In the default worker chain
([EnvironmentVariablesBackend, ExecutionAPISecretsBackend]),
ExecutionAPISecretsBackend is last — there is no later backend for the
dispatcher to fall through to, and the gap does not manifest. The
fall-through-leak scenario requires a non-default configuration: a
reordering, or a custom [secrets] backend placed after
ExecutionAPISecretsBackend. Even there, silently downgrading an
authoritative deny to "next backend, please" is wrong on principle: the
audit calls this a Type C gap — the authz control fires, but its
rejection is treated as a miss and routed around.

Fix (four parts)

1. New ErrorType.PERMISSION_DENIED

Distinct from API_SERVER_ERROR so callers can dispatch on the cause.

2. Client maps 401/403 to PERMISSION_DENIED

ConnectionOperations.get and VariableOperations.get translate 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. New AirflowSecretsBackendAccessDenied(PermissionError)

Distinct, dispatcher-aware exception class. Subclasses PermissionError
so any existing except PermissionError: handler still matches, but is
narrow enough that the dispatcher can re-raise only this signal
without accidentally promoting an incidental filesystem
OSError-family PermissionError from inside an unrelated backend.

ExecutionAPISecretsBackend (sync + async, connection + variable
variants) raises this on PERMISSION_DENIED. Other ErrorResponse
types (*_NOT_FOUND, transient API_SERVER_ERROR, GENERIC_ERROR)
continue to return None so existing recovery paths keep working.

4. Dispatchers honour the deny

The three task-SDK dispatcher loops in
airflow/sdk/execution_time/context.py (_get_connection,
_async_get_connection, _get_variable) and the two airflow-core
dispatcher loops in airflow/models/connection.py and
airflow/models/variable.py now catch
AirflowSecretsBackendAccessDenied and re-raise it before the
generic except Exception: fall-through.

Tests

  • client.connections.get / client.variables.get return
    ErrorResponse(PERMISSION_DENIED) on 401 and 403 (parametrised).
  • ExecutionAPISecretsBackend.get_connection / get_variable /
    aget_connection / aget_variable raise
    AirflowSecretsBackendAccessDenied when the response is
    PERMISSION_DENIED.
  • End-to-end dispatcher tests in
    TestDispatcherRefusesFallbackOnDeny insert a spy backend AFTER
    ExecutionAPISecretsBackend and assert it is never called once
    the first backend raises the deny — pinning the dispatcher's
    re-raise behaviour, not just the backend's. Covers
    _get_connection, _get_variable, and _async_get_connection.

Reported by

L3 ASVS sweep — apache/tooling-agents#24 (FINDING-017).


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.7)

Generated-by: Claude Code (Opus 4.7) following the guidelines

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).
@potiuk
potiukforce-pushed the fix/tasksdk-secrets-distinguish-authz-error branch from e522ad9 to 5063192CompareMay 17, 2026 19:30
@potiuk

Copy link
Copy Markdown
MemberAuthor

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)


Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

@potiukpotiuk added this to the Airflow 3.2.2 milestone May 17, 2026
@vatsrahul1001vatsrahul1001 added the ready for maintainer review Set after triaging when all criteria pass. label May 18, 2026
@vatsrahul1001

Copy link
Copy Markdown
Contributor

LGTM! ready for maintainer review

Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py Outdated
Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py
@kaxil

Copy link
Copy Markdown
Member

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)

Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

As-is, it isn't ready, check the comments above

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.
@potiuk
potiuk requested a review from XD-DENG as a code ownerMay 19, 2026 08:18
@potiukpotiuk removed the ready for maintainer review Set after triaging when all criteria pass. label May 19, 2026
@potiuk
potiuk merged commit 2b8c805 into apache:mainMay 19, 2026
113 checks passed
@potiuk
potiuk deleted the fix/tasksdk-secrets-distinguish-authz-error branch May 19, 2026 11:24
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-2-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

StatusBranchResult
v3-2-testPR Link

vatsrahul1001 pushed a commit that referenced this pull request May 19, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 21, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@potiuk@vatsrahul1001@kaxil
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Refuse secrets-backend fallback on Execution-API authz deny - #66575

Merged
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error
May 19, 2026
Merged

Refuse secrets-backend fallback on Execution-API authz deny#66575
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error

Conversation

@potiuk

@potiukpotiuk commented May 7, 2026

Copy link
Copy Markdown
Member

Summary

The Execution API's authorization decision was being routed around by the
secrets-backend dispatcher: ExecutionAPISecretsBackend.get_connection /
get_variable returned None on every ErrorResponse, conflating "not
found" with "explicitly denied". And even with the backend patched to raise
on a 401/403, the dispatcher loops in
airflow/sdk/execution_time/context.py, airflow/models/connection.py,
and airflow/models/variable.py catch Exception and continue to the
next backend — so the deny would still have been swallowed silently.

Scope of the gap

In the default worker chain
([EnvironmentVariablesBackend, ExecutionAPISecretsBackend]),
ExecutionAPISecretsBackend is last — there is no later backend for the
dispatcher to fall through to, and the gap does not manifest. The
fall-through-leak scenario requires a non-default configuration: a
reordering, or a custom [secrets] backend placed after
ExecutionAPISecretsBackend. Even there, silently downgrading an
authoritative deny to "next backend, please" is wrong on principle: the
audit calls this a Type C gap — the authz control fires, but its
rejection is treated as a miss and routed around.

Fix (four parts)

1. New ErrorType.PERMISSION_DENIED

Distinct from API_SERVER_ERROR so callers can dispatch on the cause.

2. Client maps 401/403 to PERMISSION_DENIED

ConnectionOperations.get and VariableOperations.get translate 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. New AirflowSecretsBackendAccessDenied(PermissionError)

Distinct, dispatcher-aware exception class. Subclasses PermissionError
so any existing except PermissionError: handler still matches, but is
narrow enough that the dispatcher can re-raise only this signal
without accidentally promoting an incidental filesystem
OSError-family PermissionError from inside an unrelated backend.

ExecutionAPISecretsBackend (sync + async, connection + variable
variants) raises this on PERMISSION_DENIED. Other ErrorResponse
types (*_NOT_FOUND, transient API_SERVER_ERROR, GENERIC_ERROR)
continue to return None so existing recovery paths keep working.

4. Dispatchers honour the deny

The three task-SDK dispatcher loops in
airflow/sdk/execution_time/context.py (_get_connection,
_async_get_connection, _get_variable) and the two airflow-core
dispatcher loops in airflow/models/connection.py and
airflow/models/variable.py now catch
AirflowSecretsBackendAccessDenied and re-raise it before the
generic except Exception: fall-through.

Tests

  • client.connections.get / client.variables.get return
    ErrorResponse(PERMISSION_DENIED) on 401 and 403 (parametrised).
  • ExecutionAPISecretsBackend.get_connection / get_variable /
    aget_connection / aget_variable raise
    AirflowSecretsBackendAccessDenied when the response is
    PERMISSION_DENIED.
  • End-to-end dispatcher tests in
    TestDispatcherRefusesFallbackOnDeny insert a spy backend AFTER
    ExecutionAPISecretsBackend and assert it is never called once
    the first backend raises the deny — pinning the dispatcher's
    re-raise behaviour, not just the backend's. Covers
    _get_connection, _get_variable, and _async_get_connection.

Reported by

L3 ASVS sweep — apache/tooling-agents#24 (FINDING-017).


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.7)

Generated-by: Claude Code (Opus 4.7) following the guidelines

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).
@potiuk
potiukforce-pushed the fix/tasksdk-secrets-distinguish-authz-error branch from e522ad9 to 5063192CompareMay 17, 2026 19:30
@potiuk

Copy link
Copy Markdown
MemberAuthor

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)


Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

@potiukpotiuk added this to the Airflow 3.2.2 milestone May 17, 2026
@vatsrahul1001vatsrahul1001 added the ready for maintainer review Set after triaging when all criteria pass. label May 18, 2026
@vatsrahul1001

Copy link
Copy Markdown
Contributor

LGTM! ready for maintainer review

Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py Outdated
Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py
@kaxil

Copy link
Copy Markdown
Member

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)

Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

As-is, it isn't ready, check the comments above

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.
@potiuk
potiuk requested a review from XD-DENG as a code ownerMay 19, 2026 08:18
@potiukpotiuk removed the ready for maintainer review Set after triaging when all criteria pass. label May 19, 2026
@potiuk
potiuk merged commit 2b8c805 into apache:mainMay 19, 2026
113 checks passed
@potiuk
potiuk deleted the fix/tasksdk-secrets-distinguish-authz-error branch May 19, 2026 11:24
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-2-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

StatusBranchResult
v3-2-testPR Link

vatsrahul1001 pushed a commit that referenced this pull request May 19, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 21, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@potiuk@vatsrahul1001@kaxil
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Refuse secrets-backend fallback on Execution-API authz deny - #66575

Merged
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error
May 19, 2026
Merged

Refuse secrets-backend fallback on Execution-API authz deny#66575
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error

Conversation

@potiuk

@potiukpotiuk commented May 7, 2026

Copy link
Copy Markdown
Member

Summary

The Execution API's authorization decision was being routed around by the
secrets-backend dispatcher: ExecutionAPISecretsBackend.get_connection /
get_variable returned None on every ErrorResponse, conflating "not
found" with "explicitly denied". And even with the backend patched to raise
on a 401/403, the dispatcher loops in
airflow/sdk/execution_time/context.py, airflow/models/connection.py,
and airflow/models/variable.py catch Exception and continue to the
next backend — so the deny would still have been swallowed silently.

Scope of the gap

In the default worker chain
([EnvironmentVariablesBackend, ExecutionAPISecretsBackend]),
ExecutionAPISecretsBackend is last — there is no later backend for the
dispatcher to fall through to, and the gap does not manifest. The
fall-through-leak scenario requires a non-default configuration: a
reordering, or a custom [secrets] backend placed after
ExecutionAPISecretsBackend. Even there, silently downgrading an
authoritative deny to "next backend, please" is wrong on principle: the
audit calls this a Type C gap — the authz control fires, but its
rejection is treated as a miss and routed around.

Fix (four parts)

1. New ErrorType.PERMISSION_DENIED

Distinct from API_SERVER_ERROR so callers can dispatch on the cause.

2. Client maps 401/403 to PERMISSION_DENIED

ConnectionOperations.get and VariableOperations.get translate 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. New AirflowSecretsBackendAccessDenied(PermissionError)

Distinct, dispatcher-aware exception class. Subclasses PermissionError
so any existing except PermissionError: handler still matches, but is
narrow enough that the dispatcher can re-raise only this signal
without accidentally promoting an incidental filesystem
OSError-family PermissionError from inside an unrelated backend.

ExecutionAPISecretsBackend (sync + async, connection + variable
variants) raises this on PERMISSION_DENIED. Other ErrorResponse
types (*_NOT_FOUND, transient API_SERVER_ERROR, GENERIC_ERROR)
continue to return None so existing recovery paths keep working.

4. Dispatchers honour the deny

The three task-SDK dispatcher loops in
airflow/sdk/execution_time/context.py (_get_connection,
_async_get_connection, _get_variable) and the two airflow-core
dispatcher loops in airflow/models/connection.py and
airflow/models/variable.py now catch
AirflowSecretsBackendAccessDenied and re-raise it before the
generic except Exception: fall-through.

Tests

  • client.connections.get / client.variables.get return
    ErrorResponse(PERMISSION_DENIED) on 401 and 403 (parametrised).
  • ExecutionAPISecretsBackend.get_connection / get_variable /
    aget_connection / aget_variable raise
    AirflowSecretsBackendAccessDenied when the response is
    PERMISSION_DENIED.
  • End-to-end dispatcher tests in
    TestDispatcherRefusesFallbackOnDeny insert a spy backend AFTER
    ExecutionAPISecretsBackend and assert it is never called once
    the first backend raises the deny — pinning the dispatcher's
    re-raise behaviour, not just the backend's. Covers
    _get_connection, _get_variable, and _async_get_connection.

Reported by

L3 ASVS sweep — apache/tooling-agents#24 (FINDING-017).


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.7)

Generated-by: Claude Code (Opus 4.7) following the guidelines

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).
@potiuk
potiukforce-pushed the fix/tasksdk-secrets-distinguish-authz-error branch from e522ad9 to 5063192CompareMay 17, 2026 19:30
@potiuk

Copy link
Copy Markdown
MemberAuthor

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)


Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

@potiukpotiuk added this to the Airflow 3.2.2 milestone May 17, 2026
@vatsrahul1001vatsrahul1001 added the ready for maintainer review Set after triaging when all criteria pass. label May 18, 2026
@vatsrahul1001

Copy link
Copy Markdown
Contributor

LGTM! ready for maintainer review

Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py Outdated
Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py
@kaxil

Copy link
Copy Markdown
Member

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)

Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

As-is, it isn't ready, check the comments above

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.
@potiuk
potiuk requested a review from XD-DENG as a code ownerMay 19, 2026 08:18
@potiukpotiuk removed the ready for maintainer review Set after triaging when all criteria pass. label May 19, 2026
@potiuk
potiuk merged commit 2b8c805 into apache:mainMay 19, 2026
113 checks passed
@potiuk
potiuk deleted the fix/tasksdk-secrets-distinguish-authz-error branch May 19, 2026 11:24
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-2-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

StatusBranchResult
v3-2-testPR Link

vatsrahul1001 pushed a commit that referenced this pull request May 19, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 21, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@potiuk@vatsrahul1001@kaxil
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Refuse secrets-backend fallback on Execution-API authz deny - #66575

Merged
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error
May 19, 2026
Merged

Refuse secrets-backend fallback on Execution-API authz deny#66575
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error

Conversation

@potiuk

@potiukpotiuk commented May 7, 2026

Copy link
Copy Markdown
Member

Summary

The Execution API's authorization decision was being routed around by the
secrets-backend dispatcher: ExecutionAPISecretsBackend.get_connection /
get_variable returned None on every ErrorResponse, conflating "not
found" with "explicitly denied". And even with the backend patched to raise
on a 401/403, the dispatcher loops in
airflow/sdk/execution_time/context.py, airflow/models/connection.py,
and airflow/models/variable.py catch Exception and continue to the
next backend — so the deny would still have been swallowed silently.

Scope of the gap

In the default worker chain
([EnvironmentVariablesBackend, ExecutionAPISecretsBackend]),
ExecutionAPISecretsBackend is last — there is no later backend for the
dispatcher to fall through to, and the gap does not manifest. The
fall-through-leak scenario requires a non-default configuration: a
reordering, or a custom [secrets] backend placed after
ExecutionAPISecretsBackend. Even there, silently downgrading an
authoritative deny to "next backend, please" is wrong on principle: the
audit calls this a Type C gap — the authz control fires, but its
rejection is treated as a miss and routed around.

Fix (four parts)

1. New ErrorType.PERMISSION_DENIED

Distinct from API_SERVER_ERROR so callers can dispatch on the cause.

2. Client maps 401/403 to PERMISSION_DENIED

ConnectionOperations.get and VariableOperations.get translate 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. New AirflowSecretsBackendAccessDenied(PermissionError)

Distinct, dispatcher-aware exception class. Subclasses PermissionError
so any existing except PermissionError: handler still matches, but is
narrow enough that the dispatcher can re-raise only this signal
without accidentally promoting an incidental filesystem
OSError-family PermissionError from inside an unrelated backend.

ExecutionAPISecretsBackend (sync + async, connection + variable
variants) raises this on PERMISSION_DENIED. Other ErrorResponse
types (*_NOT_FOUND, transient API_SERVER_ERROR, GENERIC_ERROR)
continue to return None so existing recovery paths keep working.

4. Dispatchers honour the deny

The three task-SDK dispatcher loops in
airflow/sdk/execution_time/context.py (_get_connection,
_async_get_connection, _get_variable) and the two airflow-core
dispatcher loops in airflow/models/connection.py and
airflow/models/variable.py now catch
AirflowSecretsBackendAccessDenied and re-raise it before the
generic except Exception: fall-through.

Tests

  • client.connections.get / client.variables.get return
    ErrorResponse(PERMISSION_DENIED) on 401 and 403 (parametrised).
  • ExecutionAPISecretsBackend.get_connection / get_variable /
    aget_connection / aget_variable raise
    AirflowSecretsBackendAccessDenied when the response is
    PERMISSION_DENIED.
  • End-to-end dispatcher tests in
    TestDispatcherRefusesFallbackOnDeny insert a spy backend AFTER
    ExecutionAPISecretsBackend and assert it is never called once
    the first backend raises the deny — pinning the dispatcher's
    re-raise behaviour, not just the backend's. Covers
    _get_connection, _get_variable, and _async_get_connection.

Reported by

L3 ASVS sweep — apache/tooling-agents#24 (FINDING-017).


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.7)

Generated-by: Claude Code (Opus 4.7) following the guidelines

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).
@potiuk
potiukforce-pushed the fix/tasksdk-secrets-distinguish-authz-error branch from e522ad9 to 5063192CompareMay 17, 2026 19:30
@potiuk

Copy link
Copy Markdown
MemberAuthor

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)


Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

@potiukpotiuk added this to the Airflow 3.2.2 milestone May 17, 2026
@vatsrahul1001vatsrahul1001 added the ready for maintainer review Set after triaging when all criteria pass. label May 18, 2026
@vatsrahul1001

Copy link
Copy Markdown
Contributor

LGTM! ready for maintainer review

Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py Outdated
Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py
@kaxil

Copy link
Copy Markdown
Member

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)

Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

As-is, it isn't ready, check the comments above

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.
@potiuk
potiuk requested a review from XD-DENG as a code ownerMay 19, 2026 08:18
@potiukpotiuk removed the ready for maintainer review Set after triaging when all criteria pass. label May 19, 2026
@potiuk
potiuk merged commit 2b8c805 into apache:mainMay 19, 2026
113 checks passed
@potiuk
potiuk deleted the fix/tasksdk-secrets-distinguish-authz-error branch May 19, 2026 11:24
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-2-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

StatusBranchResult
v3-2-testPR Link

vatsrahul1001 pushed a commit that referenced this pull request May 19, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 21, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@potiuk@vatsrahul1001@kaxil
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Refuse secrets-backend fallback on Execution-API authz deny - #66575

Merged
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error
May 19, 2026
Merged

Refuse secrets-backend fallback on Execution-API authz deny#66575
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error

Conversation

@potiuk

@potiukpotiuk commented May 7, 2026

Copy link
Copy Markdown
Member

Summary

The Execution API's authorization decision was being routed around by the
secrets-backend dispatcher: ExecutionAPISecretsBackend.get_connection /
get_variable returned None on every ErrorResponse, conflating "not
found" with "explicitly denied". And even with the backend patched to raise
on a 401/403, the dispatcher loops in
airflow/sdk/execution_time/context.py, airflow/models/connection.py,
and airflow/models/variable.py catch Exception and continue to the
next backend — so the deny would still have been swallowed silently.

Scope of the gap

In the default worker chain
([EnvironmentVariablesBackend, ExecutionAPISecretsBackend]),
ExecutionAPISecretsBackend is last — there is no later backend for the
dispatcher to fall through to, and the gap does not manifest. The
fall-through-leak scenario requires a non-default configuration: a
reordering, or a custom [secrets] backend placed after
ExecutionAPISecretsBackend. Even there, silently downgrading an
authoritative deny to "next backend, please" is wrong on principle: the
audit calls this a Type C gap — the authz control fires, but its
rejection is treated as a miss and routed around.

Fix (four parts)

1. New ErrorType.PERMISSION_DENIED

Distinct from API_SERVER_ERROR so callers can dispatch on the cause.

2. Client maps 401/403 to PERMISSION_DENIED

ConnectionOperations.get and VariableOperations.get translate 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. New AirflowSecretsBackendAccessDenied(PermissionError)

Distinct, dispatcher-aware exception class. Subclasses PermissionError
so any existing except PermissionError: handler still matches, but is
narrow enough that the dispatcher can re-raise only this signal
without accidentally promoting an incidental filesystem
OSError-family PermissionError from inside an unrelated backend.

ExecutionAPISecretsBackend (sync + async, connection + variable
variants) raises this on PERMISSION_DENIED. Other ErrorResponse
types (*_NOT_FOUND, transient API_SERVER_ERROR, GENERIC_ERROR)
continue to return None so existing recovery paths keep working.

4. Dispatchers honour the deny

The three task-SDK dispatcher loops in
airflow/sdk/execution_time/context.py (_get_connection,
_async_get_connection, _get_variable) and the two airflow-core
dispatcher loops in airflow/models/connection.py and
airflow/models/variable.py now catch
AirflowSecretsBackendAccessDenied and re-raise it before the
generic except Exception: fall-through.

Tests

  • client.connections.get / client.variables.get return
    ErrorResponse(PERMISSION_DENIED) on 401 and 403 (parametrised).
  • ExecutionAPISecretsBackend.get_connection / get_variable /
    aget_connection / aget_variable raise
    AirflowSecretsBackendAccessDenied when the response is
    PERMISSION_DENIED.
  • End-to-end dispatcher tests in
    TestDispatcherRefusesFallbackOnDeny insert a spy backend AFTER
    ExecutionAPISecretsBackend and assert it is never called once
    the first backend raises the deny — pinning the dispatcher's
    re-raise behaviour, not just the backend's. Covers
    _get_connection, _get_variable, and _async_get_connection.

Reported by

L3 ASVS sweep — apache/tooling-agents#24 (FINDING-017).


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.7)

Generated-by: Claude Code (Opus 4.7) following the guidelines

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).
@potiuk
potiukforce-pushed the fix/tasksdk-secrets-distinguish-authz-error branch from e522ad9 to 5063192CompareMay 17, 2026 19:30
@potiuk

Copy link
Copy Markdown
MemberAuthor

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)


Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

@potiukpotiuk added this to the Airflow 3.2.2 milestone May 17, 2026
@vatsrahul1001vatsrahul1001 added the ready for maintainer review Set after triaging when all criteria pass. label May 18, 2026
@vatsrahul1001

Copy link
Copy Markdown
Contributor

LGTM! ready for maintainer review

Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py Outdated
Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py
@kaxil

Copy link
Copy Markdown
Member

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)

Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

As-is, it isn't ready, check the comments above

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.
@potiuk
potiuk requested a review from XD-DENG as a code ownerMay 19, 2026 08:18
@potiukpotiuk removed the ready for maintainer review Set after triaging when all criteria pass. label May 19, 2026
@potiuk
potiuk merged commit 2b8c805 into apache:mainMay 19, 2026
113 checks passed
@potiuk
potiuk deleted the fix/tasksdk-secrets-distinguish-authz-error branch May 19, 2026 11:24
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-2-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

StatusBranchResult
v3-2-testPR Link

vatsrahul1001 pushed a commit that referenced this pull request May 19, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 21, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@potiuk@vatsrahul1001@kaxil
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Refuse secrets-backend fallback on Execution-API authz deny - #66575

Merged
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error
May 19, 2026
Merged

Refuse secrets-backend fallback on Execution-API authz deny#66575
potiuk merged 3 commits into
apache:mainfrom
potiuk:fix/tasksdk-secrets-distinguish-authz-error

Conversation

@potiuk

@potiukpotiuk commented May 7, 2026

Copy link
Copy Markdown
Member

Summary

The Execution API's authorization decision was being routed around by the
secrets-backend dispatcher: ExecutionAPISecretsBackend.get_connection /
get_variable returned None on every ErrorResponse, conflating "not
found" with "explicitly denied". And even with the backend patched to raise
on a 401/403, the dispatcher loops in
airflow/sdk/execution_time/context.py, airflow/models/connection.py,
and airflow/models/variable.py catch Exception and continue to the
next backend — so the deny would still have been swallowed silently.

Scope of the gap

In the default worker chain
([EnvironmentVariablesBackend, ExecutionAPISecretsBackend]),
ExecutionAPISecretsBackend is last — there is no later backend for the
dispatcher to fall through to, and the gap does not manifest. The
fall-through-leak scenario requires a non-default configuration: a
reordering, or a custom [secrets] backend placed after
ExecutionAPISecretsBackend. Even there, silently downgrading an
authoritative deny to "next backend, please" is wrong on principle: the
audit calls this a Type C gap — the authz control fires, but its
rejection is treated as a miss and routed around.

Fix (four parts)

1. New ErrorType.PERMISSION_DENIED

Distinct from API_SERVER_ERROR so callers can dispatch on the cause.

2. Client maps 401/403 to PERMISSION_DENIED

ConnectionOperations.get and VariableOperations.get translate 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. New AirflowSecretsBackendAccessDenied(PermissionError)

Distinct, dispatcher-aware exception class. Subclasses PermissionError
so any existing except PermissionError: handler still matches, but is
narrow enough that the dispatcher can re-raise only this signal
without accidentally promoting an incidental filesystem
OSError-family PermissionError from inside an unrelated backend.

ExecutionAPISecretsBackend (sync + async, connection + variable
variants) raises this on PERMISSION_DENIED. Other ErrorResponse
types (*_NOT_FOUND, transient API_SERVER_ERROR, GENERIC_ERROR)
continue to return None so existing recovery paths keep working.

4. Dispatchers honour the deny

The three task-SDK dispatcher loops in
airflow/sdk/execution_time/context.py (_get_connection,
_async_get_connection, _get_variable) and the two airflow-core
dispatcher loops in airflow/models/connection.py and
airflow/models/variable.py now catch
AirflowSecretsBackendAccessDenied and re-raise it before the
generic except Exception: fall-through.

Tests

  • client.connections.get / client.variables.get return
    ErrorResponse(PERMISSION_DENIED) on 401 and 403 (parametrised).
  • ExecutionAPISecretsBackend.get_connection / get_variable /
    aget_connection / aget_variable raise
    AirflowSecretsBackendAccessDenied when the response is
    PERMISSION_DENIED.
  • End-to-end dispatcher tests in
    TestDispatcherRefusesFallbackOnDeny insert a spy backend AFTER
    ExecutionAPISecretsBackend and assert it is never called once
    the first backend raises the deny — pinning the dispatcher's
    re-raise behaviour, not just the backend's. Covers
    _get_connection, _get_variable, and _async_get_connection.

Reported by

L3 ASVS sweep — apache/tooling-agents#24 (FINDING-017).


Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.7)

Generated-by: Claude Code (Opus 4.7) following the guidelines

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).
@potiuk
potiukforce-pushed the fix/tasksdk-secrets-distinguish-authz-error branch from e522ad9 to 5063192CompareMay 17, 2026 19:30
@potiuk

Copy link
Copy Markdown
MemberAuthor

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)


Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

@potiukpotiuk added this to the Airflow 3.2.2 milestone May 17, 2026
@vatsrahul1001vatsrahul1001 added the ready for maintainer review Set after triaging when all criteria pass. label May 18, 2026
@vatsrahul1001

Copy link
Copy Markdown
Contributor

LGTM! ready for maintainer review

Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py Outdated
Comment threadtask-sdk/tests/task_sdk/execution_time/test_secrets.py
@kaxil

Copy link
Copy Markdown
Member

I'd love to get this one merged — and would love it in 3.2.2 if it's not too late. cc @vatsrahul1001 (3.2.2 RM)

Drafted-by: Claude Code (Opus 4.7); reviewed by @potiuk before posting

As-is, it isn't ready, check the comments above

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.
@potiuk
potiuk requested a review from XD-DENG as a code ownerMay 19, 2026 08:18
@potiukpotiuk removed the ready for maintainer review Set after triaging when all criteria pass. label May 19, 2026
@potiuk
potiuk merged commit 2b8c805 into apache:mainMay 19, 2026
113 checks passed
@potiuk
potiuk deleted the fix/tasksdk-secrets-distinguish-authz-error branch May 19, 2026 11:24
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-2-test

Note: As of Merging PRs targeted for Airflow 3.X
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.

In matter of doubt please ask in #release-management Slack channel.

StatusBranchResult
v3-2-testPR Link

vatsrahul1001 pushed a commit that referenced this pull request May 19, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 20, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001 pushed a commit that referenced this pull request May 21, 2026
…ny (#66575) (#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 2b8c805)
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@potiuk@vatsrahul1001@kaxil