Fix recursion depth error in _redact_exception_with_context #61776

potiuk · 2026-02-11T14:11:49Z

Fix recursion depth check in _redact_exception_with_context_or_cause. There are some obscure cases where exception might point to itself in cause/context - this PR protects against it.

Changed name to include cause as well.

Initially implemented as a fix to v2-11-test in #61254 - enhanced with the case of removal of too-deep exceptions rather than not-redacting it (and replacing it with sentinel exception explaining that reminder of the stack trace has been removed.

Was generative AI tooling used to co-author this PR?

Yes (please specify the tool below)

Generated-by: Copilot + Claude Sonnet 4.5 following the guidelines

Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
When adding dependency, check compliance with the ASF 3rd Party License Policy.
For significant user-facing changes create newsfragment: {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py

shared/secrets_masker/tests/secrets_masker/test_secrets_masker.py

Fix recursion depth check in _redact_exception_with_context_or_cause. There are some obscure cases where exception might point to itself in cause/context - this PR protects against it. Changed name to include cause as well. Initially implemented as a fix to v2-11-test in apache#61254 - enhanced with the case of removal of too-deep exceptions rather than not-redacting it (and replacing it with sentinel exception explaining that reminder of the stack trace has been removed. Co-authored-by: Anton Nitochkin <[email protected]>

Copilot

Pull request overview

This PR updates the SecretsMasker exception redaction logic to safely handle circular __context__/__cause__ chains and to cap traversal using a recursion/visit limit, preventing RecursionError during log masking.

Changes:

Replace _redact_exception_with_context with _redact_exception_with_context_or_cause, adding cycle detection and a max-visit limit with a sentinel exception when truncating.
Update the log filter to call the new redaction helper.
Add extensive unit tests covering context/cause chaining, cycles, branching, and depth-limit behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
`shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py`	Introduces cycle/limit-aware exception redaction and switches the filter to use it.
`shared/secrets_masker/tests/secrets_masker/test_secrets_masker.py`	Adds new tests for the updated exception redaction behavior across multiple chaining scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-11T21:27:05Z

shared/secrets_masker/src/airflow_shared/secrets_masker/secrets_masker.py

+    def _redact_exception_with_context_or_cause(self, exception, visited=None):
        # Exception class may not be modifiable (e.g. declared by an
        # extension module such as JDBC).
        with contextlib.suppress(AttributeError):
-            exception.args = (self.redact(v) for v in exception.args)
-        if exception.__context__:
-            self._redact_exception_with_context(exception.__context__)
-        if exception.__cause__ and exception.__cause__ is not exception.__context__:
-            self._redact_exception_with_context(exception.__cause__)
+            if visited is None:
+                visited = set()


contextlib.suppress(AttributeError) currently wraps the entire traversal, not just the exception.args assignment. If an exception type has non-settable args (the comment mentions extension-module exceptions), the AttributeError will cause the function to skip redacting/truncating __context__/__cause__, potentially leaving sensitive values unredacted. Narrow the suppress(AttributeError) scope to only the exception.args = ... line (and optionally to assignments to __context__/__cause__ if needed), while keeping visited/recursion-limit logic outside the suppress block.

Copilot · 2026-02-11T21:27:05Z

shared/secrets_masker/tests/secrets_masker/test_secrets_masker.py


+    def test_redact_exception_with_context_simple(self):
+        """
+        Test _redact_exception_with_context with a simple exception without context.


This docstring/test text still refers to _redact_exception_with_context, but the method under test is now _redact_exception_with_context_or_cause. Please rename the wording here to match the new method name (and mention both context/cause if that’s what’s being exercised).

Suggested change

Test _redact_exception_with_context with a simple exception without context.

Test _redact_exception_with_context_or_cause with a simple exception without context or cause.

Copilot · 2026-02-11T21:27:06Z

shared/secrets_masker/tests/secrets_masker/test_secrets_masker.py

+    def test_redact_exception_with_max_context_recursion_depth(self):
+        """
+        Test _redact_exception_with_context respects MAX_RECURSION_DEPTH.
+        Exceptions beyond the depth limit should be skipped (not redacted).


The docstring here says exceptions beyond the depth limit are "skipped (not redacted)", but the implementation replaces the truncated tail with a sentinel exception (and drops further chaining). Update the docstring to reflect the sentinel/truncation behavior so the test intent is accurate.

Suggested change

Exceptions beyond the depth limit should be skipped (not redacted).

Once the depth limit is reached, the remaining exception chain is not traversed;

instead, it is truncated and replaced with a sentinel exception indicating the

recursion limit was hit, and further chaining is dropped.

Copilot · 2026-02-11T21:27:06Z

shared/secrets_masker/tests/secrets_masker/test_secrets_masker.py

+    def test_redact_exception_with_immutable_args(self):
+        """
+        Test _redact_exception_with_context handles exceptions with immutable args gracefully.
+        """
+        masker = SecretsMasker()


test_redact_exception_with_immutable_args only asserts the method doesn’t crash. Since the production logic should still traverse/redact __context__/__cause__ even when args can’t be set, this test won’t catch regressions where an AttributeError stops redaction of chained exceptions. Consider adding a __context__/__cause__ containing a secret and asserting it still gets redacted/truncated.

jscheffl

Wanted to have AI used again. Not too bad as review. But approved anyway.

potiuk requested review from amoghrajesh and ashb as code owners February 11, 2026 14:11

potiuk added backport-to-v3-1-test Mark PR with this label to backport to v3-1-test branch backport-to-v2-11-test Mark PR with this label to backport to v2-11-test branch labels Feb 11, 2026

potiuk requested review from hussein-awala, jscheffl and kaxil February 11, 2026 14:12

potiuk force-pushed the avoid-recursion-limit-for-exceptions-in-secrets-masker branch 2 times, most recently from 42caffe to 5326a05 Compare February 11, 2026 14:16

potiuk changed the title ~~Add comprehensive tests for _redact_exception_with_context_or_cause~~ Fix recursion depth error in _redact_exception_with_context Feb 11, 2026

potiuk mentioned this pull request Feb 11, 2026

Add handle for circular exceptions in SecretsMasker. #61254

Closed

1 task

eladkal approved these changes Feb 11, 2026

View reviewed changes

andreahlert reviewed Feb 11, 2026

View reviewed changes

potiuk force-pushed the avoid-recursion-limit-for-exceptions-in-secrets-masker branch from 5326a05 to cc6b24c Compare February 11, 2026 20:28

jscheffl requested a review from Copilot February 11, 2026 21:17

Copilot started reviewing on behalf of jscheffl February 11, 2026 21:18 View session

Copilot AI reviewed Feb 11, 2026

View reviewed changes

jscheffl approved these changes Feb 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix recursion depth error in _redact_exception_with_context #61776

Fix recursion depth error in _redact_exception_with_context #61776

potiuk commented Feb 11, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

Copilot AI Feb 11, 2026

Uh oh!

jscheffl left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	Test _redact_exception_with_context with a simple exception without context.
	Test _redact_exception_with_context_or_cause with a simple exception without context or cause.

-        Exceptions beyond the depth limit should be skipped (not redacted).
+        Once the depth limit is reached, the remaining exception chain is not traversed;
+        instead, it is truncated and replaced with a sentinel exception indicating the
+        recursion limit was hit, and further chaining is dropped.

Fix recursion depth error in _redact_exception_with_context #61776

Are you sure you want to change the base?

Fix recursion depth error in _redact_exception_with_context #61776

Conversation

potiuk commented Feb 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Was generative AI tooling used to co-author this PR?

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

Uh oh!

jscheffl left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

potiuk commented Feb 11, 2026 •

edited

Loading