AlinsRan commented on PR #13719:
URL: https://github.com/apache/apisix/pull/13719#issuecomment-5028167389
A review pass turned up that the bypass this PR closes is not just a latent
risk — a variant is **live on master today**, which strengthens the case for
this change and is worth adding to the description.
`autogenerate_admin_key()` only rewrites entries where `role == "admin"`
(core/id.lua). And the admin_key schema is written as `properties.items` rather
than `items` (cli/schema.lua), so per-entry fields are never actually
validated. So on current master:
- `admin_key: [{name: viewer, key: '', role: viewer}]` — not rewritten by
autogeneration → empty key reaches runtime → an empty `X-API-KEY:` matches that
row → unauthenticated read access to the whole Admin API. The shipped
`conf/config.yaml.example` even has this viewer entry sitting commented-out for
users to uncomment.
- `role: ops` (any non-`viewer` string) with an empty key — `check_token`'s
viewer-method restriction only fires for `role == "viewer"`, so this gives
unauthenticated full read/write.
I verified this against upstream/master directly (check_token empty-string
match, autogeneration's admin-only loop, viewer_methods = {get=true}, and the
`properties.items` schema slip). This PR's loop `util.die`s on an empty key for
**any** role, so it closes this too — worth calling out in the description, and
arguably a reason to move it out of draft sooner rather than later. The
`properties.items` schema bug is separate and pre-existing; I can file it on
its own.
--
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]
