Skip to content

Mask nested entities when writing bulk audit-log entries - #70890

Merged
potiuk merged 2 commits into
apache:mainfrom
potiuk:mask-entities-in-bulk-audit-log-entries
Aug 3, 2026
Merged

Mask nested entities when writing bulk audit-log entries#70890
potiuk merged 2 commits into
apache:mainfrom
potiuk:mask-entities-in-bulk-audit-log-entries

Conversation

@potiuk

@potiuk potiuk commented Aug 1, 2026

Copy link
Copy Markdown
Member

The audit-log maskers for Variables and Connections dispatch on top-level key
names -- val/value for variables, extra for connections. A bulk request
body has exactly one top-level key, actions, and the entities sit two levels
down in actions[].entities[], so neither masker ever saw a key it recognised
and the payload was recorded as supplied. The single-entity endpoints mask the
same fields correctly; only the bulk paths diverged.

Approach

_mask_bulk_entities walks actions[].entities[] and applies the existing
per-entity masking to each entry, so the two shapes now agree. The per-entity
logic is unchanged and simply factored out (_mask_connection_entity,
_mask_variable_entity) so both call sites share it.

  • A delete action may list bare id/key strings rather than entity objects;
    those carry nothing to mask and are passed through.
  • The helper returns None for a non-bulk body so the flat path is untouched.
  • It runs on the request body before validation, so malformed shapes
    (actions not a list, missing entities, non-dict actions) must not raise --
    each is covered by a test.

Worth noting for connections: a nested password was already covered, but only
while hide_sensitive_var_conn_fields is enabled, since that is key-name
redaction. extra was never covered by it -- the name is not a recognised
sensitive field and the value is a JSON string, which redact returns
unchanged. Masking extra here is structural and so does not depend on that
setting.

Test plan

  • TestMaskBulkFields -- bulk variables, bulk connection extra, multiple
    actions/entities, delete-by-key, and five malformed shapes
  • Verified against unmodified code: 5 of the new tests fail, and the
    guard tests correctly pass either way
  • test_decorators.py -- 30 passed
  • test_variables.py + test_connections.py -- 230 passed
  • ruff check / ruff format clean
Was generative AI tooling used to co-author this PR?
  • Yes — Claude Opus 5 (1M context)

Generated-by: Claude Opus 5 (1M context) following the guidelines at
https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions

@amoghrajesh amoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One future enhancement, otherwise LGTM.

Comment thread airflow-core/src/airflow/api_fastapi/logging/decorators.py Outdated
Comment thread airflow-core/tests/unit/api_fastapi/logging/test_decorators.py Outdated
Comment thread airflow-core/src/airflow/api_fastapi/logging/decorators.py
potiuk added 2 commits August 4, 2026 00:55
The audit-log maskers for Variables and Connections dispatch on top-level key
names. A bulk request body has exactly one top-level key, `actions`, and the
entities sit two levels down in `actions[].entities[]` -- so neither
`val`/`value` nor `extra` was ever seen, and the payload was recorded as
supplied.

Reach the entities before applying the existing per-entity masking. Bare
id/key strings in a `delete` action carry nothing to mask and are left as
they are.
The masker runs on the raw request body before validation, so extra can arrive
as any JSON type. json.loads raises TypeError rather than JSONDecodeError for a
non-string, which escaped the audit-log path entirely. Bulk bodies newly reach
this branch, so the shape is now reachable where it previously was not.
@potiuk
potiuk force-pushed the mask-entities-in-bulk-audit-log-entries branch from 21147f3 to 4450635 Compare August 3, 2026 23:00
@potiuk
potiuk merged commit fc8d6d8 into apache:main Aug 3, 2026
79 checks passed
@potiuk
potiuk deleted the mask-entities-in-bulk-audit-log-entries branch August 3, 2026 23:50
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Backport failed to create: v3-3-test. View the failure log Run details

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.

Status Branch Result
v3-3-test Commit Link

You can attempt to backport this manually by running:

cherry_picker fc8d6d8 v3-3-test

This should apply the commit to the v3-3-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.

After you have resolved the conflicts, you can continue the backport process by running:

cherry_picker --continue

If you don't have cherry-picker installed, see the installation guide.

potiuk added a commit that referenced this pull request Aug 4, 2026
…70890) (#71043)

The audit-log maskers for Variables and Connections dispatch on top-level key
names. A bulk request body has exactly one top-level key, `actions`, and the
entities sit two levels down in `actions[].entities[]` -- so neither
`val`/`value` nor `extra` was ever seen, and the payload was recorded as
supplied.

The masker also runs on the raw request body before validation, so `extra` can
arrive as any JSON type. `json.loads` raises `TypeError` rather than
`JSONDecodeError` for a non-string, which escaped the audit-log path entirely.
Bulk bodies newly reach this branch, so the shape is now reachable where it
previously was not.

(cherry picked from commit fc8d6d8)
vatsrahul1001 pushed a commit that referenced this pull request Aug 5, 2026
…70890) (#71043)

The audit-log maskers for Variables and Connections dispatch on top-level key
names. A bulk request body has exactly one top-level key, `actions`, and the
entities sit two levels down in `actions[].entities[]` -- so neither
`val`/`value` nor `extra` was ever seen, and the payload was recorded as
supplied.

The masker also runs on the raw request body before validation, so `extra` can
arrive as any JSON type. `json.loads` raises `TypeError` rather than
`JSONDecodeError` for a non-string, which escaped the audit-log path entirely.
Bulk bodies newly reach this branch, so the shape is now reachable where it
previously was not.

(cherry picked from commit fc8d6d8)
dabla pushed a commit to dabla/airflow that referenced this pull request Aug 14, 2026
* Mask nested entities when writing bulk audit-log entries

The audit-log maskers for Variables and Connections dispatch on top-level key
names. A bulk request body has exactly one top-level key, `actions`, and the
entities sit two levels down in `actions[].entities[]` -- so neither
`val`/`value` nor `extra` was ever seen, and the payload was recorded as
supplied.

Reach the entities before applying the existing per-entity masking. Bare
id/key strings in a `delete` action carry nothing to mask and are left as
they are.

* Tolerate a non-string connection extra when masking audit-log entries

The masker runs on the raw request body before validation, so extra can arrive
as any JSON type. json.loads raises TypeError rather than JSONDecodeError for a
non-string, which escaped the audit-log path entirely. Bulk bodies newly reach
this branch, so the shape is now reachable where it previously was not.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API backport-to-v3-3-test Backport to v3-3-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants