security: reconstruct exact TLS configuration boundary on integrated main - #157
Closed
seonghobae wants to merge 5 commits into
Closed
security: reconstruct exact TLS configuration boundary on integrated main#157seonghobae wants to merge 5 commits into
seonghobae wants to merge 5 commits into
Conversation
📝 WalkthroughWalkthrough
ChangesTLS 설정 보안 경계
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seonghobae
marked this pull request as ready for review
August 11, 2026 00:11
This was referenced Aug 11, 2026
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/test_tls_configuration_exact_type_documentation.py (1)
17-25: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win문서 계약에서 핵심 보안 문구를 연속된 구문으로 검증하세요.
첫 번째 테스트는
"before"와"create_ssl_context"를 독립적으로 검사합니다. 두 단어가 서로 다른 문장에 있어도 테스트가 통과할 수 있습니다.두 번째 테스트는
exact TLSConfiguration type과subclass만 검사합니다.before TLS context creation또는 인증서·호스트명 검증 문구가 삭제되어도 통과할 수 있습니다.문서 계약을 보존하려면 현재 보안 문장을 연속된 문자열로 검증하세요.
권장 검증 변경
- assert "exact `TLSConfiguration` type" in guide - assert "subclass" in guide - assert "before" in guide and "create_ssl_context" in guide - assert "hostname verification" in guide - assert "certificate verification" in guide + assert ( + "The public context helper accepts only the exact `TLSConfiguration` type " + "before it invokes `create_ssl_context()`." + ) in guide + assert ( + "a context that disables hostname verification or certificate verification" + ) in guide - assert "exact `TLSConfiguration` type" in changelog - assert "subclass" in changelog + assert ( + "Require the exact `TLSConfiguration` type before TLS context creation." + ) in changelog + assert "A subclass can no longer override `create_ssl_context()`" in changelog + assert "context that disables hostname or certificate verification" in changelogAlso applies to: 28-34
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_tls_configuration_exact_type_documentation.py` around lines 17 - 25, Update the assertions in test_tls_guide_requires_exact_configuration_type_before_dispatch and the related second test to validate the complete security wording as contiguous normalized phrases, including exact TLSConfiguration type, subclass dispatch, the requirement occurring before create_ssl_context/TLS context creation, and both hostname and certificate verification. Remove separate word-presence checks where they allow reordered or unrelated sentences to pass.tests/test_tls_configuration_exact_type.py (1)
12-28: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick win서브클래스 메서드의 호출 자체를 검증하세요.
현재
create_ssl_context()는 안전하지 않은 컨텍스트를 반환합니다. 헬퍼가 이 메서드를 먼저 호출한 후TypeError를 발생시켜도 현재 테스트는 통과합니다. 그러면 테스트가TypeError의 발생 시점까지 검증하지 못합니다.메서드가 호출되면 실패하도록 변경하거나 호출 플래그를 기록하세요. 호출 플래그가 거짓인지도 검증하세요.
검증 강화 예시
class _VerificationDisablingTLSConfiguration(TLSConfiguration): - """Return an insecure context if subclass-controlled dispatch is allowed.""" + """Fail if subclass-controlled dispatch occurs.""" def create_ssl_context(self) -> ssl.SSLContext: - """Build a context that deliberately violates the EgressWeave TLS contract.""" - context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) - context.check_hostname = False - context.verify_mode = ssl.CERT_NONE - return context + raise AssertionError("subclass dispatch must not occur")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_tls_configuration_exact_type.py` around lines 12 - 28, Strengthen test_context_helper_rejects_tls_configuration_subclasses_before_dispatch by tracking whether _VerificationDisablingTLSConfiguration.create_ssl_context was invoked, making the override fail or set a call flag when entered, and asserting the flag remains false after the expected TypeError. Keep the test focused on proving rejection occurs before subclass-controlled dispatch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_tls_configuration_exact_type_documentation.py`:
- Around line 17-25: Update the assertions in
test_tls_guide_requires_exact_configuration_type_before_dispatch and the related
second test to validate the complete security wording as contiguous normalized
phrases, including exact TLSConfiguration type, subclass dispatch, the
requirement occurring before create_ssl_context/TLS context creation, and both
hostname and certificate verification. Remove separate word-presence checks
where they allow reordered or unrelated sentences to pass.
In `@tests/test_tls_configuration_exact_type.py`:
- Around line 12-28: Strengthen
test_context_helper_rejects_tls_configuration_subclasses_before_dispatch by
tracking whether _VerificationDisablingTLSConfiguration.create_ssl_context was
invoked, making the override fail or set a call flag when entered, and asserting
the flag remains false after the expected TypeError. Keep the test focused on
proving rejection occurs before subclass-controlled dispatch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8c23eaa9-3bcc-4fca-bfdf-d6f6ca3c5bd5
📒 Files selected for processing (5)
CHANGELOG.mddocs/research/tls-configuration.mdsrc/egressweave/tls.pytests/test_tls_configuration_exact_type.pytests/test_tls_configuration_exact_type_documentation.py
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
1 participant
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.
Purpose
Reconstruct issue #111 / historical PR #148 on exact integrated protected main
dd0324c0d7edb6effc74737039d1e754af09f5aa. Historical #148 remains immutable evidence only; none of its checks, reviews, approvals, base identity, or mergeability transfers.Test-first proof
a3e387eb89e4a210bac1b3c8212250ec4cd5c607, CI31444557116: exact checkout; Ruff passed; Python 3.10 failed exactlytest_context_helper_rejects_tls_configuration_subclasses_before_dispatchwithFailed: DID NOT RAISE TypeError;1 failed, 764 passed.7b04d33c6177f62529e7473b133e9f597fd1c63e:create_egress_ssl_context()now rejects every non-exactTLSConfigurationbefore subclass-controlledcreate_ssl_context()can dispatch.1db2a43cc367f15fb0eab6b3dfd682ca5ae44919preserves the operator/research guide, migration contract, documentation regression, and[Unreleased]security history without importing stale base state.Exact-current-head evidence
On
1db2a43cc367f15fb0eab6b3dfd682ca5ae44919:31445002331: success across package acceptance and Python 3.10–3.13;767 passed;1,633/1,633statements and560/560branches;31445002332: success;31445002376: OSV, Trivy, and Scorecard success, but actual pinnedDependency reviewis skipped, so aggregate workflow success is not supply-chain acceptance.Scope
No destination authority, DNS policy, proxy behavior, request/response framing, resource limits, credentials, workflow permissions, publisher authority, or public builder signature is widened. Exact instances retain private trust, mTLS, deferred private-key password, and explicit TLS 1.2 compatibility support.
Progresses #111. Historical #148 can be closed as superseded after final ref/review-state revalidation. Merge remains blocked on the read-only central fail-closed dependency-review repair and any current review/ruleset requirements.
Summary by CodeRabbit
보안 개선
문서