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

   **Summary**
   
   Airflow's SecretsMasker decides what to hide by key name today. Values that 
reach Task logs via XCom, a print of an environment variable, a stack trace, or 
a Connection extra whose key doesn't happen to match a sensitive keyword are 
not redacted — as 
[mask-sensitive-values.rst](https://github.com/apache/airflow/blob/main/airflow-core/docs/security/secrets/mask-sensitive-values.rst)
 documents and #58514 previously reported (closed as a docs-only clarification, 
so the underlying gap remained).
   
   This PR adds an opt-in, default-off second pass in SecretsMasker that scans 
string values for a small, curated set of well-known credential formats — AWS 
access keys, GitHub / Slack / Google / Stripe tokens, PEM private-key blocks, 
JWTs — and redacts any match. Because it plugs into the existing masker, it 
runs everywhere the masker already runs (log filter, redact(), rendered fields).
   
   related: #58514
   
   **Design**
   Opt-in via [core] mask_secrets_content_patterns (default False). No existing 
deployment changes behavior.
   Seven built-in patterns, each with a distinctive fixed prefix (AKIA, AIza, 
gh[pousr]_, xox[baprs]-, sk_live_, -----BEGIN … PRIVATE KEY-----, eyJ…eyJ…) so 
match confidence is high. Formats with documented high false-positive rates in 
log data (credit cards, SSNs, emails) are deliberately left out of the defaults.
   Pluggable. SecretsMasker.add_content_patterns({name: regex}) registers 
deployment-specific formats; invalid regexes are logged and skipped, not raised.
   
   ReDoS-audited. Every pattern uses fixed-width or bounded quantifiers, no 
nested quantifiers, no overlapping alternations, and no \S*? + 
negative-lookahead combo (the shape fixed in #70716). Worst case is a single 
linear scan.
   Extends SecretsMasker rather than adding a parallel class — the masker is 
already a logging.Filter shared by airflow-core and the task-SDK via 
shared/secrets_masker/.
   The log-record filter() short-circuit was widened from self.replacer to 
self.replacer or self.mask_content_patterns, otherwise content-pattern masking 
would silently no-op in log paths when no explicit add_mask() secrets are 
registered.
   
   **Compatibility**
   No behavior change with the flag off.
   Not a replacement for mask_secret() — this is defense in depth. Known 
secrets should still be registered explicitly.
   No new runtime dependencies, no schema change, no new decorator.
   
   **Testing**
   25 new tests covering each built-in pattern, false-positive guardrails, 
add_content_patterns() registration and invalid-regex rejection, reset_masker() 
behavior, composition with existing key-name masking and with add_mask(), and 
the log-filter path when only content-pattern masking is enabled.
   Full pre-existing suite: 167 passed, 1 skipped (pre-existing k8s import), 1 
xfailed (pre-existing) — no regressions.
   ruff format and ruff check clean.
   
   **Related**
   related: #58514 — same limitation, closed as docs-only clarification.
   Independent context: #70716 (ReDoS fix in Spark provider) — reviewed my 
regexes against the same class of issue. #70890 (bulk audit-log masking) — 
different code path, no conflict.
   
   **Reviewer notes**
   Happy to trim the initial pattern set (e.g. drop JWTs, which have the 
highest false-positive rate of the seven) or flip the default in a follow-up PR 
once we have adoption data. Not adding a newsfragment yet; can add one on 
request.
   
   Was generative AI tooling used to co-author this PR?
   Yes — Cursor Agent (Opus 4.7)


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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to