Skip to content

security(tls): require exact trust scalar values - #214

Draft
seonghobae wants to merge 8 commits into
mainfrom
security/tls-config-scalar-integrity-main-7faf
Draft

security(tls): require exact trust scalar values#214
seonghobae wants to merge 8 commits into
mainfrom
security/tls-config-scalar-integrity-main-7faf

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Purpose

Progress #213 by repairing the retained TLS trust, identity, and direct private-key-password scalar boundary on protected main@7faf7a3b8a47980113982914000e724ab6a6cda5.

The exact outer TLSConfiguration type is already protected-main behavior. This bounded follow-up prevents path text, inline CA material, and direct non-callback password values from carrying built-in-subclass behavior into frozen declarative state. The zero-argument password callback remains an explicit trusted integration point.

Test-first lineage

Path, trust, and CA scalars

  • RED d523c640703cd322f45fb52ce58652b6d2fccfec: hostile strip() / __len__() behavior reproduced in hosted CI;
  • GREEN 1442fdb89b8b931992bc92d84f6ae9dcfba43801: exact built-in scalar boundary applied.

Direct private-key-password scalars

  • RED 00f75595571d8be2445da146d78dd17cffd301f6: hostile text/bytes retention and bytearray conversion reproduced;
  • GREEN d3be1c0bb7ae11d2c8095b7730e5164a4c414920: exact direct-password scalar boundary and immutable bytearray copy applied;
  • documentation alignment 6ecc1b2e2caa1af5293a28a5d07a5eddda6f0327 records the boundary in the TLS research/operator contract.

Current exact identity

  • live/protected base: main@7faf7a3b8a47980113982914000e724ab6a6cda5;
  • exact current head: 0bfc22b48e7e7b13e32d2b3795a17c3c19c8c576;
  • state: open / Draft / mechanically mergeable;
  • current delta: TLS implementation, focused regression coverage, TLS research documentation, and root CHANGELOG.md only.

The current tree:

  • observes path-like input through os.fspath() exactly once and accepts only an exact built-in str result;
  • requires ca_data to be exact built-in str or bytes before inspection or retention;
  • requires direct private-key passwords to be exact built-in str, bytes, or bytearray, copying an exact bytearray to immutable bytes;
  • preserves the explicit zero-argument callback contract for deferred trusted secret retrieval;
  • preserves exact paths, PEM text / DER bytes, pathlib.Path, private trust, mTLS, TLS 1.3 default, explicit TLS 1.2 compatibility, hostname/certificate verification, exact outer configuration type, and public builders;
  • performs no path expansion/resolution, filesystem access, runtime network call, credential logging, persistence, business authorization, or egress-authority widening during immutable construction.

Review repair

CodeRabbit's current finding requiring an [Unreleased] entry for the TLS scalar security boundary was revalidated against the current tree. The former same-path defer to #212 is no longer an active-writer constraint: #212 is Draft and its CHANGELOG.md branch has not moved since 2026-08-13.

The current head now records the accepted/normalized trust-path, ca_data, and direct private-key-password values, built-in-subclass rejection, immutable bytearray copy, and caller migration path in the root changelog. The corresponding review thread PRRT_kwDOTVyu-86Yx2nP is resolved. A post-write diff audit confirmed that the only CHANGELOG.md delta is this six-line security entry; unrelated historical text is preserved.

Fresh governance and evidence state

The active organization ruleset 18156473 currently requires one qualifying approval, dismisses stale reviews on push, requires review-thread resolution, and requires the central Close Empty PR, OpenCode, PR Review Merge Scheduler, Security Scan, Strix, SAST Semgrep, and Noema workflows. OrganizationAdmin bypass exists technically but is not an accepted merge path.

The exact current head was created by the bounded changelog repair, so predecessor-head CI/security/review evidence does not transfer. New exact-head CI, Security Scan, and SAST Semgrep runs have materialized and are currently queued; queued evidence is non-passing.

The prior exact-head Security Scan also exposed a separate organization-owned supply-chain evidence defect: the wrapper could succeed while the immutable Dependency Review action was skipped. Central .github#897 currently owns the fail-closed repair and is an active writer lane, so this repository does not duplicate or race that control-plane work. A fresh Security Scan must execute the real pinned Dependency Review action successfully before this PR can leave Draft.

Design-system applicability

Fresh repository search finds no Storybook implementation or Figma reference in EgressWeave. This PR is a provider-neutral Python transport/security library boundary with no buyer-facing UI flow, so no Figma/Storybook artifact is fabricated for this change.

Remaining acceptance

  1. Require all exact-current-head repository and central required workflows to reach terminal success; queued, skipped-required, stale, or predecessor evidence is non-passing.
  2. Require a fresh Security Scan whose immutable-pinned Dependency Review action actually executes and succeeds after the central fail-closed control is integrated.
  3. Require zero valid unresolved findings and one qualifying independent non-author human approval under the live ruleset.
  4. Immediately before integration, refetch exact head, live base, ancestry, ruleset, required workflows, review threads, formal reviews, and active-writer evidence.
  5. Merge only the unchanged accepted tree through normal protection. Do not self-approve, use OrganizationAdmin bypass, synthesize review/check evidence, or create a no-op commit solely to retrigger external behavior.

Keep Draft until these conditions are satisfied.

Summary by CodeRabbit

  • 개선 사항

    • TLS 설정에서 경로, 인증서 데이터, 개인 키 비밀번호에 허용되는 내장 타입을 명확히 했습니다.
    • bytearray 비밀번호는 안전하게 불변 값으로 복사해 보관합니다.
    • 비밀번호 콜백을 통한 지연 조회를 계속 지원합니다.
    • 신뢰 저장소와 상호 TLS 설정에도 동일한 타입 규칙을 적용했습니다.
  • 문서

    • TLS 설정의 타입 요구사항과 지원 범위를 상세히 갱신했습니다.
  • 테스트

    • 문자열·바이트·경로 객체·콜백 및 타입 서브클래스 처리에 대한 회귀 테스트를 추가했습니다.

@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

TLS 설정의 경로, CA 데이터, 개인 키 비밀번호에 정확한 내장 타입 검사를 적용했습니다. bytearray 비밀번호는 bytes로 복사합니다. 문서와 회귀 테스트는 새 계약과 기존 지원 입력을 반영합니다.

Changes

TLS 스칼라 무결성

Layer / File(s) Summary
정확한 내장 타입 검증 및 비밀번호 정규화
src/egressweave/tls.py, docs/research/tls-configuration.md
경로와 CA 데이터는 정확한 내장 타입만 허용합니다. 개인 키 비밀번호는 정확한 str, bytes, bytearray 또는 callable만 허용합니다. bytearraybytes로 복사합니다.
TLS 스칼라 회귀 검증
tests/test_tls_configuration_scalar_integrity.py
경로, CA 데이터, 비밀번호의 내장 타입 서브클래스를 거부하는 동작을 검증합니다. 표준 입력, pathlib.Path, 콜백, bytearray 복사도 검증합니다.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Mergeability Score: 🔵 Low · up to 6ecc1

The change narrows accepted TLS scalar inputs and preserves trusted callback-based secret retrieval. The implementation is otherwise ready, but the required CHANGELOG update documenting the security-boundary and caller-impact change remains before merge.

Possibly related issues

  • 이슈 213: tls.py의 정확한 타입 검증, 문서 변경, 회귀 테스트가 이슈의 목표를 직접 구현합니다.

Possibly related PRs

  • ContextualWisdomLab/EgressWeave#172: 두 PR 모두 TLS 타입 경계를 강화합니다. 이 PR은 경로·CA·비밀번호 스칼라를 다루고, 해당 PR은 TLSConfiguration 인스턴스 검증을 다룹니다.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목은 TLS 신뢰 관련 스칼라 값에 정확한 내장 타입을 요구하는 핵심 변경을 명확하게 설명합니다. 직접 제공되는 개인 키 비밀번호 변경까지는 명시하지 않지만, 변경사항의 주요 보안 목적과 일치합니다.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch security/tls-config-scalar-integrity-main-7faf

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@docs/research/tls-configuration.md`:
- Around line 43-61: Update CHANGELOG.md to document the TLS scalar
security-boundary changes reflected by the surrounding documentation: accepted
and normalized types for trust paths, ca_data, and direct private-key passwords;
rejection of subclasses; copying exact bytearray passwords; and the resulting
caller migration impact. Mark this changelog entry as the remaining release
acceptance task.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bea3fddb-6c5d-4936-b76e-4ffd28805df0

📥 Commits

Reviewing files that changed from the base of the PR and between 7faf7a3 and 6ecc1b2.

📒 Files selected for processing (3)
  • docs/research/tls-configuration.md
  • src/egressweave/tls.py
  • tests/test_tls_configuration_scalar_integrity.py

Comment thread docs/research/tls-configuration.md
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