Eason09053360 opened a new pull request, #72468:
URL: https://github.com/apache/airflow/pull/72468
## Why
`Credentials.load()` guarded only against the credentials file being absent.
Every other way that file can be unusable reached the operator as a stack
trace, because `safe_call_command` recognises none of those exception types:
| file state | before |
| --- | --- |
| wrong permissions | `PermissionError` |
| non-UTF-8 bytes | `UnicodeDecodeError` |
| truncated or empty | `JSONDecodeError` |
| valid JSON, wrong shape | `TypeError` |
| no `api_url` key | `KeyError: 'api_url'` |
`$AIRFLOW_HOME/production.json` is a generic name in a shared directory, so
a foreign or half-written file is not far-fetched. The traceback was also a
dead end: the fix is to log in again, but `auth login` loads the same file and
failed the same way, leaving deleting the file by hand as the only way out.
## What
`_read_cli_config(path)` in `airflow-ctl/src/airflowctl/api/client.py` takes
over the open and the parse, which is what puts all of those cases in one place
— `OSError` for unreadable paths, `ValueError` for both malformed JSON and
non-UTF-8 bytes (siblings, not parent and child), plus an explicit check that
the parsed result is a JSON object with an `api_url`. They raise
`AirflowCtlCredentialInvalidException`, a new `AirflowCtlException` subclass.
`FileNotFoundError` is re-raised untouched so callers keep handling an absent
file as the distinct case it is, and the debug-credentials read goes through
the same helper.
`load()` extends the tolerance it already had for a missing file to an
unusable one, so `auth login` — the recovery the message recommends — can run
and replace it.
`safe_call_command` catches `AirflowCtlException` rather than a
hand-maintained tuple of subclasses. The tuple was already equivalent to the
base class, and keeping it manual means the next exception class added anywhere
regresses to a traceback, which is the failure this PR is fixing. One side
effect: the path-traversal guard in `_safe_path_under_airflow_home` raises
`AirflowCtlException` directly and so was not caught before; it now renders as
a message.
Tests cover each of the file states above and that `AUTH` / `NO_AUTH`
clients still load past an unusable file.
Deliberately out of scope: `{"api_url": null}` still loads as `None` and
lets `get_client` fall back to `localhost:8080`. That is a
silently-wrong-server bug rather than a traceback, `Credentials.save()` itself
writes such a file, and an existing test pins the current behaviour — it wants
its own change. `auth list-envs` also reads the same file through its own
logic, which now defines "unusable" slightly differently.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Opus 5)
Generated-by: Claude Code (Opus 5) following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
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]
