FrankYang0529 opened a new pull request, #72683:
URL: https://github.com/apache/airflow/pull/72683
## Why
- `action_logging` is a FastAPI dependency, and dependencies are solved
before Pydantic validates the request body. It sees whatever `json.loads`
produced, so a list, string or number reaches it as valid JSON.
- Two of its branches read that body as a mapping via `request_body.items()`
with no type guard, so `POST /api/v2/variables` and `POST /api/v2/connections`
raise `AttributeError` and return 500 instead of the 422 the OpenAPI spec
declares.
- `action_logging` commits its audit row before the endpoint runs, so an
attempt is recorded even when the request is later rejected. The crash happens
before that commit, so on `main` these requests leave no audit row at all.
## How
- Assign the parsed body to `request_body` only when it is a dict, so the
variable keeps its `{}` initial value for every other shape.
## Verification
- Unit test: `uv run --frozen --project airflow-core pytest
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_connections.py
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_variables.py`
- Integration test:
1. Start breeze
```sh
breeze shell --backend sqlite
```
2. Start an API server
```sh
export AIRFLOW__CORE__LOAD_EXAMPLES=False
airflow db migrate
airflow api-server --port 8080 &
until curl -sf -o /dev/null http://localhost:8080/api/v2/version; do sleep
2; done
```
3. Get a token
```sh
TOKEN=$(curl -s -X POST http://localhost:8080/auth/token \
-H 'Content-Type: application/json' \
-d '{"username": "admin", "password": "admin"}' \
| python -c 'import sys, json;
print(json.load(sys.stdin)["access_token"])')
```
4. POST a non-dict JSON body to both endpoints
```sh
for path in variables connections; do
for body in '[{"key": "a"}]' '"a string"' '42'; do
echo -n "POST /$path <- $body => "
curl -s -o /tmp/body.json -w 'HTTP %{http_code}\n' \
-X POST "http://localhost:8080/api/v2/$path"; \
-H "Authorization: Bearer $TOKEN" -H 'Content-Type: application/json'
-d "$body"
head -c 200 /tmp/body.json; echo
done
done
```
Before this PR, all six requests return `HTTP 500` with a bare `Internal
Server Error`.
With this PR they return the 422 the OpenAPI spec declares:
```
POST /variables <- [{"key": "a"}] => HTTP 422
{"detail":[{"type":"model_attributes_type","loc":["body"],
"msg":"Input should be a valid dictionary or object to extract fields
from","input":[{"key":"a"}]}]}
```
5. Confirm valid bodies are unaffected
```sh
curl -s -w '\nHTTP %{http_code}\n' -X POST
http://localhost:8080/api/v2/variables \
-H "Authorization: Bearer $TOKEN" -H 'Content-Type: application/json' \
-d '{"key": "demo", "value": "s3cr3t", "description": "d"}'
curl -s -w '\nHTTP %{http_code}\n' -X POST
http://localhost:8080/api/v2/connections \
-H "Authorization: Bearer $TOKEN" -H 'Content-Type: application/json' \
-d '{"connection_id": "demo", "conn_type": "http", "password": "p4ss",
"extra": "{\"token\": \"abc\"}"}'
```
6. Check the audit log
```sh
curl -s "http://localhost:8080/api/v2/eventLogs?limit=100"; -H
"Authorization: Bearer $TOKEN" \
| python -c '
import sys, json
for r in json.load(sys.stdin)["event_logs"]:
print("id=%-3s event=%-16s owner=%-6s extra=%s" % (r["event_log_id"],
r["event"], r["owner"], r["extra"]))
```
Before this PR, the six rejected requests leave **no audit rows at all** —
`action_logging` raises before reaching its `session.commit()`, so the record
of the attempt is lost.
With this PR every attempt is recorded:
```
id=1 event=cli_api_server owner=root extra={"host_name": "...",
"full_command": "..."}
id=2 event=post_variable owner=admin extra={"method": "POST"}
id=3 event=post_variable owner=admin extra={"method": "POST"}
id=4 event=post_variable owner=admin extra={"method": "POST"}
id=5 event=post_connection owner=admin extra={"method": "POST"}
id=6 event=post_connection owner=admin extra={"method": "POST"}
id=7 event=post_connection owner=admin extra={"method": "POST"}
id=8 event=post_variable owner=admin extra={"key": "demo", "value":
"***", "description": "d", "method": "POST"}
id=9 event=post_connection owner=admin extra={"connection_id": "demo",
"conn_type": "http", "password": "***", "extra": {"token": "***"}, "method":
"POST"}
```
<!-- SPDX-License-Identifier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->
<!--
Thank you for contributing!
Please provide above a brief description of the changes made in this pull
request.
Write a good git commit message following this guide:
https://chris.beams.io/posts/git-commit/
Please make sure that your code changes are covered with tests.
And in case of new features or big changes remember to adjust the
documentation.
For user-facing UI changes, please attach before/after screenshots (or a
short
screen recording) so reviewers can assess the visual impact.
Feel free to ping (in general) for the review if you do not see reaction for
a few days
(72 Hours is the minimum reaction time you can expect from volunteers) - we
sometimes miss notifications.
In case of an existing issue, reference it using one of the following:
* closes: #ISSUE
* related: #ISSUE
-->
---
##### Was generative AI tooling used to co-author this PR?
<!--
If generative AI tooling has been used in the process of authoring this PR,
please
change below checkbox to `[X]` followed by the name of the tool, uncomment
the "Generated-by".
-->
- [X] Yes - Claude Code
<!--
Generated-by: [Tool Name] following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
-->
---
* Read the **[Pull Request
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)**
for more information. Note: commit author/co-author name and email in commits
become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals))
is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License
Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment:
`{pr_number}.significant.rst`, in
[airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
You can add this file in a follow-up commit after the PR is created so you
know the PR number.
--
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]
