fix(api): default bind to 127.0.0.1 and warn on non-loopback without API_AUTH_KEY#338
Merged
Merged
Conversation
Robin1987China
force-pushed
the
safe-bind-default
branch
from
bf15a7b to
ef83cfe
Compare
…API_AUTH_KEY Default the --host bind address to 127.0.0.1 instead of 0.0.0.0 to match the secure-by-default posture already used in docker-compose. Add a startup warning when binding to a non-loopback address without API_AUTH_KEY configured — the existing loopback peer-IP and DNS-rebinding checks still reject remote requests, but the warning surfaces the misconfiguration at startup. Also default the Typer `serve` command's --host to 127.0.0.1 so every serve entry point agrees (the console script and `python -m cli` already bound loopback; this covers the `python -m cli.main` path too). Tests: _is_loopback_bind_host truth table (IPv4/IPv6/hostname/edge), loopback default, and the warning firing only on a non-loopback bind without a configured key. Refs HKUDS#333 Signed-off-by: Robin1987China <41602358+Robin1987China@users.noreply.github.com>
Robin1987China
force-pushed
the
safe-bind-default
branch
from
ef83cfe to
9c83d6b
Compare
Contributor
|
LGTM |
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Default the
--hostbind address from0.0.0.0to127.0.0.1, and log a startup warning when binding to a non-loopback address withoutAPI_AUTH_KEYconfigured.Why
The Docker compose already binds to
127.0.0.1:8899:8899, so loopback-only is the intended secure-by-default posture — but the bare CLI default of0.0.0.0did not match. The existing loopback peer-IP and DNS-rebinding checks still reject remote requests, but defense-in-depth argues the bind itself should default to loopback. Binding to all interfaces should be a conscious opt-in, with a warning when done withoutAPI_AUTH_KEY.Changes
agent/api_server.py:--hostdefault"0.0.0.0"→"127.0.0.1"; added_is_loopback_bind_host()(ipaddress.is_loopback+"localhost"fallback); startup warning when binding non-loopback withoutAPI_AUTH_KEY.agent/cli/main.py: Typerservecommand--hostdefault also set to127.0.0.1, so every serve entry point agrees. The console script andpython -m clialready bound loopback (they route through the argparse path); this covers thepython -m cli.mainTyper path too.agent/tests/test_serve_bind.py: new test coverage.Docker is unaffected: the Dockerfile passes
--host 0.0.0.0explicitly and compose binds loopback on the host side — neither relies on the default.I left out one reviewer-style nit — only computing the warning state lazily — because the check is a single comparison at startup, not a hot path.
Test Plan
api_server.pypre-exist on origin/main; none introduced)_is_loopback_bind_hosttruth table incl. IPv6/hostname/edge, loopback default, warning only on non-loopback without a key)Checklist
Refs #333