mlevkov opened a new pull request, #3804:
URL: https://github.com/apache/iggy/pull/3804
Addresses suggestions **2 and 3** of #3802. Deliberately does **not** close
it —
suggestion 1 (redacting or gating the config responses) needs a maintainer
decision, explained at the bottom.
## The gap
Two defaults compose badly. `api_key = ""` means `resolve_api_key` waves
every
request through:
```rust
if context.api_key.expose_secret().is_empty() {
return Ok(next.run(request).await);
}
```
and the configuration routes return plugin configuration exactly as parsed
from
TOML — a database connection string, an S3 secret key, a webhook signing
secret.
There is no redaction anywhere in `core/connectors/runtime/src`.
The shipped `address = "127.0.0.1:8081"` confines that to local processes,
which
is a defensible posture for an admin API and is why this is a hardening gap
rather than a disclosure. The problem is what happens next: changing
`address` to
`0.0.0.0` to reach the API from outside a container is an ordinary thing to
do,
and it silently turns "local processes can read the credentials" into "the
network can", with no signal at any layer.
## What this changes
**A startup warning** when the API is enabled, `api_key` is empty, and the
address resolves beyond loopback. Warns rather than refuses to start —
refusing
would break deployments that are exposed today, and that is the maintainers'
call, not mine.
It **resolves** the address rather than parsing it, which matters more than
it
looks: `HttpConfig::default()` is `localhost:8081`, which is loopback but is
not
a `SocketAddr`. A parse-based check would fire on the default config and
train
operators to ignore the warning. An address that cannot resolve counts as
exposed, since it is about to fail the bind regardless.
Four unit tests cover the decision table (loopback / IPv6 loopback /
`localhost`
/ `0.0.0.0` / a routable IP / key configured / unresolvable). These are the
first
tests in `src/api/`, so they add a `mod tests` there. Mutation-checked:
dropping
the `api_key` early return fails the configured-key case.
**Documentation.** The endpoint list said what each route returns but never
that
the config routes return credentials, so a reader had no way to know
exposing the
port exposes their secrets. Added an admonition under `## HTTP API`, and the
`api_key` comment in `config.toml` now says that empty disables
authentication
rather than just calling the key "optional" — plus a note on why the default
address is loopback, which otherwise reads as an arbitrary default rather
than
the control doing the confining.
## Why suggestion 1 is not here
"Redact credential-bearing fields in the config responses" cannot be done
faithfully at this layer: `plugin_config` is an opaque `serde_json::Value`,
so the
runtime has no way to know which keys are credentials. A key-name heuristic
(`*password*`, `*token*`, `*secret*`, `*key*`) would both miss fields and
redact
innocent ones, and silently returning altered config from an API operators
may
read programmatically is its own hazard.
The alternative in the issue — requiring `api_key` for those specific routes
regardless of the global default — is a clean fix but a breaking behaviour
change
for anyone consuming those routes unauthenticated today.
Both are defensible; picking between them is a maintainer decision, so I have
left #3802 open rather than guess. Happy to implement either.
Related: #3801, which is why the plugin-side `SecretString` annotations do
not
help here — the runtime never routes through them.
## Verification
`cargo fmt --all --check`, `cargo sort --check --no-format --workspace`,
`cargo clippy -p iggy-connectors --all-features --all-targets -- -D
warnings`,
`cargo test -p iggy-connectors` (126 pass), `taplo fmt --check`, `hawkeye
check`,
`typos`, `markdownlint` — all exit 0.
--
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]
