Repository navigation
Mask CLI secrets in the audit log by parsed argument, not argv position - #72650
Merged
Merged
Conversation
potiuk
force-pushed
the
security/cli-audit-log-masking
branch
from
September 7, 2026 18:24
e8d3aa3 to
c4ab1d4
Compare
potiuk
force-pushed
the
security/cli-audit-log-masking
branch
from
September 27, 2026 13:20
f020041 to
753b3ae
Compare
_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
force-pushed
the
security/cli-audit-log-masking
branch
from
October 4, 2026 23:26
753b3ae to
2028c73
Compare
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
…udit log Generated-by: Claude Opus 5
Generated-by: Claude Opus 5
Generated-by: Claude Opus 5
…arsed credentials Generated-by: Claude Opus 5
Generated-by: Claude Opus 5
Generated-by: Claude Opus 5
Generated-by: Claude Opus 5
vatsrahul1001
approved these changes
Oct 18, 2026
Contributor
There was a problem hiding this comment.
Looks good to me overall. Taking the values from the parsed namespace fixes the positional masking and the added coverage is useful. Could use reviews from @amoghrajesh @kaxil before merging.
The command line is masked for the audit log before the command runs, so an unexpected error in the masking aborted every CLI command. A failure now records the command fully masked instead: the command still runs, and nothing a secret could be in reaches the log. The short-option table is also checked against the real parsers, so a command definition change cannot silently change what gets masked. Generated-by: Claude Opus 5
Contributor
Backport successfully created: v3-4-testNote: As of Merging PRs targeted for Airflow 3.X In matter of doubt please ask in #release-management Slack channel.
|
potiuk
added a commit
that referenced
this pull request
Oct 19, 2026
… argv position (#72650) (#75144) * Mask CLI secrets in the audit log by parsed argument, not argv position _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. * Mask long-form password options on every CLI subcommand in the audit log --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 * Mask CLI secrets by parsed value in every argument spelling 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 secrets after grouped short flags and in task-params / env-vars 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 * Limit in-token masking to whole values and short-option clusters 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 * Mask clustered, repeated-empty and double-scheme URI secrets in CLI audit log Generated-by: Claude Opus 5 * Only treat a URI as double-scheme when the host carries a protocol Generated-by: Claude Opus 5 * Match Connection's double-scheme parsing when masking URIs Generated-by: Claude Opus 5 * Walk short-option clusters by option arity and fail closed on any unparsed credentials Generated-by: Claude Opus 5 * Mask the whole URI when credentials cannot be located reliably Generated-by: Claude Opus 5 * Detect percent-encoded credentials when masking URIs Generated-by: Claude Opus 5 * Match credentials across newlines when masking URIs Generated-by: Claude Opus 5 * Keep CLI commands running when audit-log masking fails The command line is masked for the audit log before the command runs, so an unexpected error in the masking aborted every CLI command. A failure now records the command fully masked instead: the command still runs, and nothing a secret could be in reaches the log. The short-option table is also checked against the real parsers, so a command definition change cannot silently change what gets masked. (cherry picked from commit 4bbe5f5) Generated-by: Claude Opus 5 Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
vatsrahul1001
pushed a commit
that referenced
this pull request
Oct 20, 2026
… argv position (#72650) (#75144) * Mask CLI secrets in the audit log by parsed argument, not argv position _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. * Mask long-form password options on every CLI subcommand in the audit log --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 * Mask CLI secrets by parsed value in every argument spelling 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 secrets after grouped short flags and in task-params / env-vars 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 * Limit in-token masking to whole values and short-option clusters 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 * Mask clustered, repeated-empty and double-scheme URI secrets in CLI audit log Generated-by: Claude Opus 5 * Only treat a URI as double-scheme when the host carries a protocol Generated-by: Claude Opus 5 * Match Connection's double-scheme parsing when masking URIs Generated-by: Claude Opus 5 * Walk short-option clusters by option arity and fail closed on any unparsed credentials Generated-by: Claude Opus 5 * Mask the whole URI when credentials cannot be located reliably Generated-by: Claude Opus 5 * Detect percent-encoded credentials when masking URIs Generated-by: Claude Opus 5 * Match credentials across newlines when masking URIs Generated-by: Claude Opus 5 * Keep CLI commands running when audit-log masking fails The command line is masked for the audit log before the command runs, so an unexpected error in the masking aborted every CLI command. A failure now records the command fully masked instead: the command still runs, and nothing a secret could be in reaches the log. The short-option table is also checked against the real parsers, so a command definition change cannot silently change what gets masked. (cherry picked from commit 4bbe5f5) Generated-by: Claude Opus 5 Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
_build_metricsrecordssys.argvinto the audit log (Log.extra) and maskedthe
variablesvalue by position, assuming the key and value were the finaltwo arguments.
Any supported trailing option displaces them.
airflow variables set KEY VALUE --description TEXTends in--description TEXT, so the value was written to theaudit log in clear.
--descriptionis the instance, not the class — the same istrue of
--serialize-jsonand of any option added later.The change
Namespaceand redact that valuewherever it appears in the recorded command. Position no longer matters.
--conn-extrais now masked. It was not handled at all, and it isprecisely the field connection secrets live in.
--conn-jsonand--conn-extraare walked recursively.--conn-jsonmasked only top-level keys, so a secret one level down was recorded in clear.
--conn-json,--conn-extraand--conn-uriwere matched as standalone tokens only, so the equally valid--conn-json=VALUEform skipped redaction entirely.--conn-urihad that gapbefore this change too.
--passwordand
--conn-passwordwere masked only underusersandconnections, soprovider-contributed subcommands that take a password
(
airflow keycloak-auth-manager create-scopes --password ...) recorded it inclear.
-pstays limited tousersandconnections, since elsewhere itmeans
--portor--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.extrarows and backupsretain values already written and should be redacted or expired, and any
credentials exposed that way rotated.
Tests
Seven new cases: the value displaced by
--descriptionand 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-extraand--conn-uri. All seven fail against unpatched sources; the existing cases areunchanged and still pass. Three more cover the password options:
keycloak-auth-manager --passwordin both spellings is masked, andapi-server -p 8080is recorded as given.Not addressed here
airflow dags trigger --confandairflow backfill create --dag-run-confalsoaccept 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?
Generated-by: Claude Code following the guidelines