Skip to content

fix(api): add DNS rebinding protection via Host header validation#124

Merged
haofeif merged 4 commits into
mainfrom
fix/dns-rebinding-host-validation
Mar 17, 2026
Merged

fix(api): add DNS rebinding protection via Host header validation#124
haofeif merged 4 commits into
mainfrom
fix/dns-rebinding-host-validation

Conversation

@fanhongy

Copy link
Copy Markdown
Contributor

🔒 Security Fix: DNS Rebinding Protection (CVE Mitigation)

Severity: HIGH (CVSS 8.1)
Type: DNS Rebinding Attack / Command Injection
Status: FIXED

Summary

Adds DNS rebinding protection to the CAO server by implementing Host header validation via FastAPI's TrustedHostMiddleware. Without this fix, malicious websites could exploit DNS rebinding to send arbitrary prompts to running CAO agents, potentially executing commands, reading sensitive files, or exfiltrating credentials.

Vulnerability Description

CAO server did not validate the Host header in incoming HTTP requests. This allowed DNS rebinding attacks where:

  1. User visits malicious website (attack.poc)
  2. Website's DNS initially points to attacker's server
  3. Attacker's DNS changes attack.poc127.0.0.1 (short TTL)
  4. Browser allows JavaScript requests to attack.poc (same-origin)
  5. Requests actually go to localhost:9889 (CAO server)
  6. Attacker can enumerate sessions, inject prompts, execute commands, and read terminal output

Attack Vector: Network (requires user to visit malicious site)
Impact: Command execution, data exfiltration, credential theft

Changes Made

File Change
src/cli_agent_orchestrator/constants.py Added ALLOWED_HOSTS (localhost, 127.0.0.1, ::1)
src/cli_agent_orchestrator/api/main.py Added TrustedHostMiddleware
test/api/test_security.py New: 24 security tests (DNS rebinding scenarios, attack simulation)
test/api/conftest.py New: TestClientWithHost fixture for middleware compatibility
test/api/test_terminals.py Removed duplicate client fixture (uses conftest)
test/api/test_inbox_messages.py Removed duplicate client fixture (uses conftest)

Net Addition: ~350 lines (mostly security tests)

Testing

$ uv run pytest test/api/test_security.py -v
======================== 22 passed, 2 skipped in 2.70s ========================

$ uv run pytest test/api/ -v
======================== 54 passed, 2 skipped in 0.46s ========================

✅ Legitimate requests (localhost, 127.0.0.1) → 200 OK
❌ Attack requests (attack.poc, malicious-site.com) → 400 Bad Request

No Breaking Changes

Normal CAO usage (server, CLI, MCP connections, browser access on localhost) is completely unaffected.

References


⚠️ This is a security patch that should be prioritized for release.

@haofeif haofeif added the bug Something isn't working label Mar 17, 2026
@haofeif

haofeif commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

@fanhongy Good security fix — DNS rebinding is a real risk for localhost services like CAO, and TrustedHostMiddleware is the right approach.

Items to fix

1. Code Quality CI is failingblack wants to reformat constants.py and test_security.py. Please run:

uv run black src/cli_agent_orchestrator/constants.py test/api/test_security.py

2. IPv6 tests are skipped — Both [::1] and ::1 tests have @pytest.mark.skip. Since you added these to ALLOWED_HOSTS, they should either be tested or removed from the allowed list. Untested code paths in security-critical config are a concern.

@fanhongy

Copy link
Copy Markdown
Contributor Author

Right. removed ipv6 from ALLOWED_HOSTS.

@haofeif

haofeif commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

Right. removed ipv6 from ALLOWED_HOSTS.

Thanks again for the quick turnaround @fanhongy . Some suggestions (minor) below :

1. Add a note to README.md Security section

Users who expose the server (--host 0.0.0.0) will be confused when requests from other machines are rejected. Please add a brief note to the existing Security section in README.md:

The server validates HTTP `Host` headers to prevent [DNS rebinding attacks](https://owasp.org/www-community/attacks/DNS_Rebinding).
Only `localhost` and `127.0.0.1` are accepted by default — requests with other hostnames are rejected with `400 Bad Request`.

2. Add a comment in constants.py for future extensibility

The allowed hosts list is currently hardcoded, which is correct for now. But please add a comment noting the intended extension point so future maintainers know where to add CLI/env
support:

# Allowed Host headers for DNS rebinding protection (CVE mitigation)
# Only localhost connections permitted - CAO is a local-only service
# These hosts are validated by TrustedHostMiddleware to prevent DNS rebinding attacks
# Note: IPv6 (::1) is not included as CAO is accessed via IPv4 localhost in practice
# To allow additional hosts in the future, add a --allowed-hosts CLI flag
# or CAO_ALLOWED_HOSTS env var (comma-separated).
ALLOWED_HOSTS = [
    "localhost",
    "127.0.0.1",
]

Rest LGTM, much appreciated again~

@fanhongy

Copy link
Copy Markdown
Contributor Author

added to README.md with a bit statement and comment on the ALLOW_HOSTS

@haofeif haofeif merged commit 2a86bef into main Mar 17, 2026
10 checks passed
@haofeif haofeif deleted the fix/dns-rebinding-host-validation branch March 17, 2026 23:16
call-me-ram added a commit to call-me-ram/cli-agent-orchestrator that referenced this pull request Jun 3, 2026
Bring the event-driven architecture branch up to date with main (98
commits) and reconcile the rewrite with features that landed after it
forked: eager inbox delivery (awslabs#251), the OpenCode poller, env-var
forwarding (awslabs#259), memory curation (awslabs#254/awslabs#262), CORS auto-derive (awslabs#261),
DNS host validation (awslabs#124), and the self-send guard (awslabs#24).

Highlights:
- Providers adopt the async initialize() + get_status(buffer) contract;
  copilot_cli/opencode_cli converted; kiro keeps colour-only ANSI
  stripping so carriage-return-redraw permission prompts aren't misread
  as idle.
- Event-driven InboxService.deliver_pending with the awslabs#251 eager gate and
  message-sender attribution; OpenCode poller retained as a status-driven
  method; the watchdog (PollingObserver/LogFileHandler) is removed.
- terminal_service.create_terminal is async (FIFO + StatusMonitor wiring);
  session_service.create_session, flow_service.execute_flow, the API
  endpoints, and `cao flow run` updated to await.
- memory_service curated path and the flow CLI fixed to the new contract.

Full unit suite green (1908 passed); black + isort clean.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants