ShousenZHANG opened a new pull request, #72986:
URL: https://github.com/apache/airflow/pull/72986

   `init_wsgi_middlewares.py` and `init_security.py` are both listed in 
`OVERLOOKED_TESTS` and have no tests at all, yet `create_app()` calls both 
unguarded — `init_api_auth(flask_app)` at 
`providers/fab/src/airflow/providers/fab/www/app.py:114` and 
`init_wsgi_middleware(flask_app)` at `:131`. A regression in the first stops 
the app from starting; one in the second silently changes which proxy headers 
are trusted.
   
   Both are in one PR rather than one each because it is the same review.
   
   ### What the tests pin
   
   **`init_wsgi_middleware`** builds `ProxyFix` from five separate `[fab] 
proxy_fix_x_*` options passed as five near-identical keyword arguments:
   
   - the five options are set to five *different* values, so swapping any two 
of those arguments fails the test
   - with `enable_proxy_fix` off, `wsgi_app` is left alone
   - with it on and nothing else configured, all five counts are 1 — Werkzeug's 
own defaults for `x_host`, `x_port` and `x_prefix` are 0, so this pins that 
Airflow deliberately trusts every header once
   
   **`init_api_auth`** splits `[fab] auth_backends` on commas and imports each 
entry:
   
   - whitespace after a comma is stripped before importing, and the backends 
are imported in order and each gets `init_app(app)`
   - with the option unset, the session backend is the one imported
   - an unimportable backend surfaces as `AirflowException`, not `ImportError`
   - a failure partway through the list leaves the backends imported so far on 
`app.api_auth`; recorded as current behaviour, since the exception propagates 
out of `create_app` and the half-populated app is never served. Happy to drop 
that one if you would rather not pin it.
   
   ### Mutation results
   
   Assertion count says nothing about whether a test can fail, so each module 
was broken one line at a time to check. 9 of 10 mutations are caught:
   
   | mutation | caught |
   | --- | --- |
   | `x_host` reads `PROXY_FIX_X_PORT` instead | yes |
   | drop the `x_prefix=` keyword (Werkzeug then defaults it to 0) | yes |
   | invert the `ENABLE_PROXY_FIX` guard | yes |
   | hardcode `x_for=1`, ignoring the option | yes |
   | drop `.strip()` before `import_module` | yes |
   | swallow the `ImportError` instead of raising | yes |
   | stop appending to `app.api_auth` | yes |
   | change the registered `proxy_fix_x_host` default | yes |
   | change the registered `auth_backends` default | yes |
   | change the `fallback=` string in `init_security.py` | **no** |
   
   The last one is not a gap in the tests — it is unreachable code. Both 
modules pass `fallback=` to `conf.get`/`conf.getint`, but every one of those 
options has a registered provider default, so the registered value wins and the 
`fallback=` argument can never be used. I left it alone rather than widen this 
PR; happy to file it separately if it is worth removing.
   
   ### Scope
   
   This clears 2 of the 7 `fab/www/extensions` entries in `OVERLOOKED_TESTS`. 
The rest are deliberately left: `test_init_session.py` is already being added 
by #72825, and `init_appbuilder`, `init_views`, `init_jinja_globals` and 
`init_manifest_files` each need more setup than these two and suit their own 
PRs. So the block does not reach zero here.
   
   Worth noting that #72610 *removed* dead helpers from this same directory 
rather than testing them, so I checked the call sites before writing anything — 
both functions under test are still reached from `create_app()` at the lines 
given above.
   
   ### Verification
   
   - `pytest providers/fab/tests/unit/fab/www/extensions/` — 7 passed
   - `pytest airflow-core/tests/unit/always/test_project_structure.py` — 10 
passed, 1 pre-existing xfail
   - `ruff check` and `ruff format --check` on the new files — clean
   - `prek run --files <the 4 changed files>` — 44 passed, 211 skipped, 0 failed
   
   ##### Was generative AI tooling used to co-author this PR?
   
   - [X] Yes — Claude Code
   
   Generated-by: Claude Code following [the 
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
   
   The tests were written by an AI assistant. I checked the call sites in 
`www/app.py` myself, ran the mutation table above to confirm each test can 
actually fail, and ran the commands listed under Verification.
   


-- 
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]

Reply via email to