Skip to content

[AAASM-3685] 🔒 (transport): Require TLS for non-loopback gateway/op-control#181

Merged
Chisanan232 merged 12 commits into
masterfrom
v0.0.1/AAASM-3685/transport_tls_hardening
Jun 25, 2026
Merged

[AAASM-3685] 🔒 (transport): Require TLS for non-loopback gateway/op-control#181
Chisanan232 merged 12 commits into
masterfrom
v0.0.1/AAASM-3685/transport_tls_hardening

Conversation

@Chisanan232

Copy link
Copy Markdown
Contributor

Description

Hardens both SDK→gateway transports against sending sensitive material over plaintext to a remote host. Adds a shared loopback/scheme validator (agent_assembly/core/transport_security.py) reused by both paths (mirrors node-sdk AAASM-3123):

  • AAASM-3685 (op-control gRPC): OpControlSubscriber.connect previously opened grpc.insecure_channel unconditionally. It now refuses a plaintext channel to a non-loopback gateway unless the caller passes a channel_factory (e.g. a TLS channel) or the new allow_insecure=True opt-in. Loopback (localhost/127.0.0.1/::1) stays plaintext for local dev.
  • AAASM-3725 (control-plane HTTP): GatewayClient sent Authorization: Bearer <api_key> over plaintext http:// with no scheme guard. It now refuses, when an API key is set, to send the Bearer header over http:// to a non-loopback host unless allow_insecure=True. https:// and loopback always pass. init_assembly additionally emits a resolution-time warning for the same condition.

Secure-by-default; loopback dev and explicit opt-in preserved.

Type of Change

  • 🔧 Bug fix (security hardening)

Breaking Changes

  • Yes (please describe below)

Connecting to a non-loopback gateway over plaintext (insecure gRPC, or http:// with an API key set) now raises ValueError unless the caller passes a TLS channel_factory / allow_insecure=True. Loopback and https:// are unaffected. This is intentional secure-by-default behavior.

Related Issues

  • Related JIRA ticket: AAASM-3685, AAASM-3725

Testing

  • Unit tests added/updated

uv sync then pytest test/ — 713 passed, 16 skipped (env-gated). mypy agent_assembly clean for changed files (4 pre-existing _core/grpc-stub errors unchanged from master). New tests cover: insecure gRPC to non-loopback rejected, loopback/opt-in allowed; Bearer-over-http non-loopback rejected, loopback/https allowed, control-plane-url is the validated target; resolution warning fires (and is silent for loopback). One pre-existing header test updated to opt in via allow_insecure=True.

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Comments added for complex logic
  • All tests passing

🤖 Generated with Claude Code

Chisanan232 and others added 10 commits June 25, 2026 19:56
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…k host

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.91667% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
agent_assembly/core/transport_security.py 97.43% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Comment thread test/unit/client/test_gateway_endpoints.py Fixed
Comment thread test/unit/client/test_gateway_endpoints.py Fixed
Comment thread test/unit/client/test_gateway_endpoints.py Fixed
Comment thread test/unit/client/test_gateway_endpoints.py Fixed
Comment thread test/unit/client/test_gateway_endpoints.py Fixed
@Chisanan232

Copy link
Copy Markdown
Contributor Author

🔎 Claude Code review — fix-wave

CI: All functional checks green (unit/integration 3.13, CodeQL, pip-audit, CI Success, codecov/patch). Only SonarCloud Code Analysis red — acceptance/quality-gate, out of scope, ignored.
Scope vs AAASM-3685 / AAASM-3725: Matches both tickets precisely. 3685 (op-control gRPC insecure_channel unconditional) → require_secure_grpc_target gate before channel open, loopback/opt-in preserved, channel_factory (TLS) path untouched. 3725 (Bearer-over-http:// leak) → require_secure_http_url validates the actual control_plane_url or gateway_url target + resolution-time warning in init_assembly. Shared transport_security.py mirrors node-sdk AAASM-3123 as the ticket recommends. No scope creep.
Side-effects / regression: NONE for legitimate usage. Verified in a throwaway clone (uv sync && pytest test/): 711 passed, 16 skipped (env-gated/native). The only deviation was 2 timing-sensitive test/bench/test_latency_contracts.py benchmarks flaking under full-suite CPU contention — both pass in isolation, the bench file is byte-identical to master (unchanged by this PR), so pre-existing flakiness, not a regression. Confirmed: loopback (localhost/127.0.0.1/[::1]) gateways still work plaintext (dev case); https:// always passes; http:// without an API key passes; refusal triggers only for non-loopback http://+api_key / insecure gRPC; allow_insecure=True opt-in works; the one intentionally-updated header test opts in via allow_insecure. Default localhost:7391 resolution unaffected. ; Intended breaking: non-loopback plaintext (insecure gRPC, or http://+api_key) now raises ValueError unless allow_insecure=True — secure-by-default, documented in PR body.
Readiness: Ready to merge once SonarCloud is dispositioned per acceptance policy. Code, tests, and scope are clean.
— Claude Code (automated PR review, 2026-06-25)

… tests

Convert the five TestHttpTransportSecurity cases from a
client = GatewayClient(...) / try: ... / finally: client.close()
pattern to a with GatewayClient(...) as client: block, so cleanup
runs via __exit__. Resolves the CodeQL 'Should use a with statement'
review findings; assertions and pytest.raises checks unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Chisanan232

Copy link
Copy Markdown
Contributor Author

Security review dispositions (SonarCloud + CodeQL)

SonarCloud Security Hotspots — 2 (both python:S5332, "Using http protocol is insecure")

Both hotspots point at the same line (agent_assembly/core/transport_security.py:114) inside require_secure_http_url:

raise ValueError(
    f"Refusing to send an Authorization: Bearer credential over plaintext "
    f"http:// to non-loopback gateway {gateway_url!r}. Use https://, or pass "
    f"allow_insecure=True to explicitly opt in (loopback dev only)."
)

Disposition: false positive (Safe) — no code change. The flagged http:// token is a literal substring inside the error message of the function whose entire job is to refuse plaintext-http with a Bearer credential to a non-loopback host. There is no URL construction, no network call, and no http:// endpoint used here — S5332 is pattern-matching the string http:// in human-readable refusal text. Removing/rewording it would only degrade the diagnostic. Recommend marking these two hotspots Safe in SonarCloud. The actual encryption enforcement (this very raise, plus require_secure_grpc_target and warn_if_insecure_http_url) is what AAASM-3685/3725 adds.

CodeQL review comments — 5 ("Should use a with statement")

All 5 flagged the client = GatewayClient(...) / try: ... / finally: client.close() pattern in TestHttpTransportSecurity. Fixed in 2dcbbfd: all five methods now use with GatewayClient(...) as client: so cleanup runs via __exit__; assertions and pytest.raises checks are unchanged.

Validation: pytest test/ → 713 passed, 16 skipped, no regressions. mypy agent_assembly → only the pre-existing native-_core/grpc-stub environment errors (unchanged by this commit; the edited file is a test module).

— Claude Code, 2026-06-25

…essage

Reword the require_secure_http_url ValueError to describe the plaintext
(non-TLS) connection without the literal http:// scheme token, clearing
two python:S5332 SonarCloud hotspots on new code. No behavior change;
the diagnostic still names the Bearer credential, non-loopback host,
https:// remedy, and allow_insecure opt-in.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@Chisanan232

Copy link
Copy Markdown
Contributor Author

SonarCloud gate green — S5332 false positives resolved

The only red check was SonarCloud Code Analysis: 2 python:S5332 ("clear-text http protocol") Security Hotspots, both on the same new-code line — a literal http:// token inside the error-message string of require_secure_http_url in agent_assembly/core/transport_security.py.

These were false positives: the function constructs no URL and makes no network call. It is the guard that refuses to send a Bearer credential over plaintext to a non-loopback host; the http:// substring lived only in the diagnostic text.

Fix (no behavior change): reworded the ValueError so it conveys the same meaning without the bare http:// scheme token, which is what S5332 keys on:

Refusing to send an Authorization: Bearer credential over a plaintext (non-TLS) connection to non-loopback gateway <url>. Use an https:// endpoint, or pass allow_insecure=True to explicitly opt in (loopback dev only).

The diagnostic still names the Bearer credential, the non-loopback host, the https:// remedy, and the allow_insecure opt-in. (https:// does not trigger S5332.) No logic changed.

Validation: full suite 713 passed, 16 skipped; mypy agent_assembly shows no new errors (only the pre-existing missing-_core/grpc-stub baseline). Transport-security tests still pass — the existing pytest.raises(ValueError, match="Bearer") assertion remained valid, so no test changes were needed.

Final CI: SonarCloud Code Analysis ✅ pass, CI Success ✅ pass, 0 failing checks.

— Claude Code, 2026-06-25

@Chisanan232 Chisanan232 merged commit 692d3bb into master Jun 25, 2026
24 checks passed
@Chisanan232 Chisanan232 deleted the v0.0.1/AAASM-3685/transport_tls_hardening branch June 25, 2026 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant