lxbme commented on issue #3416:
URL:
https://github.com/apache/apisix-dashboard/issues/3416#issuecomment-5111900426
I went through these four items before starting on a fix, and re-verified
each against `master` (`e76036c2`) with APISIX 3.17.0 (`apache/apisix:dev`),
since the report was written against `9979b31`. Three findings, one of which
changes the suggested implementation.
## 1 and 3 — confirmed
Reproduced end to end. Seeding a credential:
```
PUT /apisix/admin/consumers/sec_demo/credentials/cred-keyauth
{"plugins":{"key-auth":{"key":"SUPER-SECRET-KEY-12345"}}}
```
then opening the credential detail page and clicking **View** on the
key-auth card renders `"SUPER-SECRET-KEY-12345"` verbatim in the drawer's DOM,
in read-only view mode. There is no masking concept anywhere in
`PluginEditorDrawer`.
Worth noting explicitly, because it makes the case stronger rather than
weaker: APISIX *does* encrypt these fields at rest (a `PUT` response returns
the ciphertext), but the Admin API decrypts them on `GET`. So the dashboard
genuinely receives plaintext, and the dashboard is where the exposure happens —
masking here is the right layer, not a workaround for something the gateway
should be doing.
## 2 — I could not reproduce this on 3.17.0
The Admin API does not appear to return SSL private keys at all. Creating an
SSL with a `cert`/`key` pair and reading it back:
```
GET /apisix/admin/ssls/masktest
→ value keys: ['cert', 'create_time', 'id', 'sni', 'update_time']
```
No `key`, and no `keys` on the list endpoint either. The detail page's
"Private Key 1" field renders **empty** as a result — the certificate shows
(correctly; it is public), the private key has nothing to show.
A side effect of the same behaviour, which may be worth its own issue:
because the key is never returned, editing an SSL and saving fails with `400
invalid configuration: then clause did not match` unless the private key is
re-pasted, since the schema requires `cert` and `key` together.
`e2e/tests/ssls.crud-required-fields.spec.ts` already works around exactly this
("Click Cancel instead of Save to avoid validation issues with empty key").
If this was observed on an older APISIX that did return the key, that would
explain the difference — happy to be corrected with a version where it
reproduces.
## 4 — not reproduced either
I tried three malformed payloads hoping to get a validation error that
echoed the submitted value. The closest was:
```
{"plugins":{"key-auth":{"key":123456789}}}
→ {"error_msg":"... property \"key\" validation failed: wrong type: expected
string, got number"}
```
The message names the field and the type, not the value. That is only three
probes against one plugin, so it does not disprove the concern — but if you
have a case where a secret does come back in `error_msg`, that would pin down
whether the fix belongs in the toast or is broader.
## On the suggested implementation
The report proposes maintaining a per-plugin sensitive-path registry in the
dashboard. I would push back on that, because **the gateway already publishes
exactly that list**, and the dashboard is already fetching it.
`GET /apisix/admin/plugins?all=true` — the request that already drives the
plugin editor — returns `encrypt_fields` per plugin, in both `schema` and
`consumer_schema`. On 3.17.0 that is 40 plugins, including every path named in
this issue:
| plugin | `encrypt_fields` |
| --- | --- |
| `key-auth` | `["key"]` (consumer_schema) |
| `basic-auth` | `["password"]` (consumer_schema) |
| `jwt-auth` | `["secret"]` (consumer_schema) |
| `hmac-auth` | `["secret_key"]` (consumer_schema) |
| `limit-count` | `["redis_password", "sentinel_password"]` |
| `openid-connect` | `["client_secret", "client_rsa_private_key",
"session.secret", "session.redis.password"]` |
| `ai-proxy` | `["auth.header", "auth.query",
"auth.gcp.service_account_json", "auth.aws.secret_access_key",
"auth.aws.session_token"]` |
| `kafka-logger` | `["brokers.sasl_config.password"]` |
…plus `clickhouse-logger`, `elasticsearch-logger`, `splunk-hec-logging`,
`aws-lambda`, `azure-functions`, `authz-keycloak`, `saml-auth`,
`google-cloud-logging`, and others.
The format is already dotted paths, which is what a redaction transform
needs. Deriving the mask from `encrypt_fields` instead of a hand-maintained
list means no drift as plugins change, coverage of third-party plugins that
declare it, and no extra request — the data is in a response the dashboard
already has.
So the shape I would suggest for 1 and 3 is: in view mode, redact the paths
the plugin's own schema marks as encrypted, with a reveal affordance; leave
edit mode as-is.
Does that direction sound right? And on item 2 — is there a version or path
where the private key does come back? I would rather not add masking to a field
that is structurally always empty.
--
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]
