Skip to content

Mask CLI secrets in the audit log by parsed argument, not argv position - #72650

Open
potiuk wants to merge 5 commits into
apache:mainfrom
potiuk:security/cli-audit-log-masking
Open

potiuk wants to merge 5 commits into
apache:mainfrom
potiuk:security/cli-audit-log-masking

Conversation

@potiuk

@potiuk potiuk commented Sep 7, 2026 •

Copy link
Copy Markdown
Member

_build_metrics records sys.argv into the audit log (Log.extra) and masked
the variables value by position, assuming the key and value were the final
two arguments.

Any supported trailing option displaces them. airflow variables set KEY VALUE --description TEXT ends in --description TEXT, so the value was written to the
audit log in clear. --description is the instance, not the class — the same is
true of --serialize-json and of any option added later.

The change

  • Take the key and value from the parsed Namespace and redact that value
    wherever it appears in the recorded command. Position no longer matters.
  • --conn-extra is now masked. It was not handled at all, and it is
    precisely the field connection secrets live in.
  • --conn-json and --conn-extra are walked recursively. --conn-json
    masked only top-level keys, so a secret one level down was recorded in clear.
  • Both argparse spellings are handled. --conn-json, --conn-extra and
    --conn-uri were matched as standalone tokens only, so the equally valid
    --conn-json=VALUE form skipped redaction entirely. --conn-uri had that gap
    before this change too.
  • Long-form password options are masked on every subcommand. --password
    and --conn-password were masked only under users and connections, so
    provider-contributed subcommands that take a password
    (airflow keycloak-auth-manager create-scopes --password ...) recorded it in
    clear. -p stays limited to users and connections, since elsewhere it
    means --port or --exclude-providers.

A value that cannot be parsed as JSON is redacted whole: these options carry
connection material, and a value that cannot be inspected is not safe to record.

For operators

This prevents new disclosures only. Existing Log.extra rows and backups
retain values already written
and should be redacted or expired, and any
credentials exposed that way rotated.

Tests

Seven new cases: the value displaced by --description and by --serialize-json,
a non-sensitive key that must still be recorded as given, nested --conn-extra,
nested --conn-json, and the = spelling for --conn-json, --conn-extra and
--conn-uri. All seven fail against unpatched sources; the existing cases are
unchanged and still pass. Three more cover the password options: keycloak-auth-manager --password in both spellings is masked, and api-server -p 8080 is recorded as given.

Not addressed here

airflow dags trigger --conf and airflow backfill create --dag-run-conf also
accept JSON that reaches the audit log unmasked. That is a real and separate
pre-existing exposure with a different blast radius, and folding it in would
widen this change beyond the defect it was scoped to. Worth its own issue.


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: Claude Code following the guidelines

@potiuk
potiuk force-pushed the security/cli-audit-log-masking branch from e8d3aa3 to c4ab1d4 Compare September 7, 2026 18:24
@potiuk
potiuk force-pushed the security/cli-audit-log-masking branch from f020041 to 753b3ae Compare September 27, 2026 13:20
potiuk added 2 commits October 5, 2026 01:17
_build_metrics recorded sys.argv into the audit log and masked the variables
value by position, assuming the key and value were the final two arguments. Any
supported trailing option displaces them: 'airflow variables set KEY VALUE
--description TEXT' ends in '--description TEXT', so the value was written to
Log.extra in clear. --description is the instance found, not the class -- the
same is true of --serialize-json and of any option added later.

Take the key and value from the parsed Namespace instead, and redact that value
wherever it appears in the recorded command. Position no longer matters.

--conn-extra was not masked at all. It is the field connection secrets live in,
and it accepts nested JSON. It is now redacted alongside --conn-json, and both
are walked recursively -- --conn-json previously masked only top-level keys, so
a secret one level down was recorded in clear. A value that cannot be parsed as
JSON is redacted whole, because these options carry connection material and a
value that cannot be inspected is not safe to record.

Both argparse spellings are handled. --conn-json, --conn-extra and --conn-uri
were matched as standalone tokens only, so the equally valid --conn-json=VALUE
form skipped redaction entirely and recorded the secret. --conn-uri had that gap
before this change as well.

Note for operators: this prevents new disclosures only. Existing Log.extra rows
and backups retain values already written and should be redacted or expired,
and any credentials exposed that way rotated.

The seven new cases fail against unpatched sources.
--password and --conn-password name a password on every subcommand,
including those contributed by providers (keycloak-auth-manager ...
--password), but were only masked for users and connections. Mask the
long spellings everywhere; keep -p limited to users and connections, as
elsewhere it means --port or --exclude-providers.

Generated-by: Claude Opus 5
@potiuk
potiuk force-pushed the security/cli-audit-log-masking branch from 753b3ae to 2028c73 Compare October 4, 2026 23:26
@potiuk
potiuk requested a review from amoghrajesh October 13, 2026 14:27
@potiuk potiuk added the backport-to-v3-4-test Backport to v3-4-test label Oct 16, 2026
@potiuk potiuk added this to the Airflow 3.4.0 milestone Oct 16, 2026
@potiuk

potiuk commented Oct 16, 2026

Copy link
Copy Markdown
Member Author

@amoghrajesh @kaxil Could you take a look at this when you have a moment? It's targeted at 3.4.0 and needs a review to make the cut on 2026-10-19. CI is green.

Mask secrets supplied through any supported argument spelling in the
recorded command line: abbreviated and attached options, repeated
options, JSON (incl. conf) and URI values, and celery flower auth.

Generated-by: Claude Opus 5 following the guidelines at https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions
Mask a parsed secret wherever it appears in a recorded token, which covers grouped
short flags, and add tasks test -t/--task-params and --env-vars to the JSON options.

Generated-by: Claude Opus 5 following the guidelines at https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions
Mask a parsed value only where it is a whole token or ends a single-dash short-option
cluster, for plain, JSON and URI options alike, so a short value is not masked wherever
its text happens to appear in the recorded command line.

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-to-v3-4-test Backport to v3-4-test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants