fix(postgres): require authenticated TLS for remote pg8000 - #342
seonghobae wants to merge 28 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough원격 pg8000 연결에 엄격한 TLS 서버 신원 검증을 추가했습니다. 루프백 연결은 기존 예외를 유지합니다. TLS 검증 실패와 정확한 SSL 거부 오류는 내용 없는 정책 오류로 변환합니다. 단위 테스트와 실제 PostgreSQL 수락 테스트를 추가했습니다. Changes원격 TLS 정책
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~45 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant SmokeTest
participant Pg8000DriverAdapter
participant pg8000
participant PostgreSQL
SmokeTest->>Pg8000DriverAdapter: 원격 주소와 CA 설정 전달
Pg8000DriverAdapter->>pg8000: 검증된 ssl_context로 연결
pg8000->>PostgreSQL: TLS 핸드셰이크 수행
PostgreSQL-->>pg8000: 인증 결과 반환
pg8000-->>Pg8000DriverAdapter: 연결 또는 TLS 오류 반환
Pg8000DriverAdapter-->>SmokeTest: 세션 또는 Pg8000DriverTlsPolicyError 반환
Merge Risk: ⚪ Minimal · up to Remote connections enforce authenticated TLS while loopback retains its explicit development exception, with successful CI and release acceptance coverage. 🚥 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 |
|
@coderabbitai review Please review the exact current head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected='7f6864cd4fb94ef9a0e76955b06babad990c8c00'
actual="$(git rev-parse HEAD)"
printf 'HEAD expected=%s actual=%s\n' "$expected" "$actual"
git status --short
printf '\n--- relative diff against parent ---\n'
git diff --find-renames --find-copies --stat HEAD^ HEAD
git diff --find-renames --find-copies --name-status HEAD^ HEAD
git diff --find-renames --find-copies --no-ext-diff --unified=80 HEAD^ HEAD
printf '\n--- candidate files ---\n'
fd -t f . | rg '(pg8000|postgres|Postgres|tls|TLS|requirements|pyproject|poetry\.lock|Pipfile|setup\.cfg|setup\.py)$' || true
printf '\n--- Python structure maps ---\n'
fd -e py -t f . | rg '(pg8000|postgres|Postgres|tls|TLS)' | while IFS= read -r file; do
printf '\n### %s\n' "$file"
ast-grep outline "$file" --items all || true
done
printf '\n--- Connection and SSL call contexts ---\n'
rg -n -C 8 --glob '*.py' \
'(pg8000|PostgresDriverPort|ssl_context|SSLContext|create_default_context|check_hostname|CERT_REQUIRED|connect\s*\()' \
. || true
printf '\n--- dependency declarations ---\n'
rg -n -C 3 '(pg8000|postgres)' \
pyproject.toml poetry.lock requirements.txt requirements-*.txt setup.py setup.cfg Pipfile Pipfile.lock 2>/dev/null || trueLength of output: 201769 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- exact commit and two-file delta ---'
git rev-parse HEAD
git diff --name-status HEAD^ HEAD
git diff --no-ext-diff --unified=20 HEAD^ HEAD -- \
pg_llm_batch/pg8000_driver_adapter.py \
tests/test_pg8000_remote_tls_identity.py
printf '%s\n' '--- current production adapter ---'
cat -n pg_llm_batch/pg8000_driver_adapter.py
printf '%s\n' '--- parent production adapter ---'
git show HEAD^:pg_llm_batch/pg8000_driver_adapter.py | cat -n
printf '%s\n' '--- focused TLS test ---'
cat -n tests/test_pg8000_remote_tls_identity.py
printf '%s\n' '--- inherited adapter connect implementation ---'
ast-grep outline pg_llm_batch/pg8000_candidate_driver_port.py --items all
rg -n -A100 -B10 '^\s*def connect\(' pg_llm_batch/pg8000_candidate_driver_port.py
printf '%s\n' '--- production adapter construction and direct candidate constructions ---'
rg -n -C 5 --glob '*.py' \
'(load_pg8000_driver\s*\(|Pg8000DriverAdapter\s*\(|Pg8000CandidateDriverAdapter\s*\()' \
pg_llm_batch tests
printf '%s\n' '--- focused test outcomes from repository CI metadata ---'
gh pr view 342 --repo ContextualWisdomLab/pg-llm-batch --json headRefOid,statusCheckRollup,isDraft,reviewDecision \
--jq '{headRefOid,isDraft,reviewDecision,statusCheckRollup:[.statusCheckRollup[]|{name,conclusion,status}]}'Length of output: 76062 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='b68a975becc93841dbfea818beccb8119f2f6944'
head='7f6864cd4fb94ef9a0e76955b06babad990c8c00'
printf '%s\n' '--- ancestry and relative delta from the stated source parent ---'
git merge-base --is-ancestor "$base" "$head" && echo 'source parent is an ancestor'
git log --oneline --decorate "$base..$head"
git diff --stat "$base" "$head"
git diff --name-status "$base" "$head"
git diff --no-ext-diff --unified=30 "$base" "$head" -- \
pg_llm_batch/pg8000_driver_adapter.py \
tests/test_pg8000_remote_tls_identity.py
printf '%s\n' '--- validated host and service-resolution boundaries ---'
sed -n '110,175p' pg_llm_batch/pg8000_candidate_driver_port.py
sed -n '270,405p' pg_llm_batch/pg8000_candidate_driver_port.py
ast-grep outline pg_llm_batch/pg8000_candidate_service_file.py --items all
rg -n -A120 -B10 'def __call__|def resolve|class Pg8000CandidateServiceFileResolver' \
pg_llm_batch/pg8000_candidate_service_file.py
printf '%s\n' '--- failed check details ---'
gh pr checks 342 --repo ContextualWisdomLab/pg-llm-batch || true
gh pr view 342 --repo ContextualWisdomLab/pg-llm-batch --json statusCheckRollup \
--jq '.statusCheckRollup[] | select(.name == "Coverage, docstrings, lint, and package") | {detailsUrl,workflowName,name,conclusion,status}'Length of output: 31116 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- failed hosted gate log ---'
gh run view 34561425189 \
--repo ContextualWisdomLab/pg-llm-batch \
--job 103144768662 \
--log-failed || true
printf '%s\n' '--- local static check for TLS policy mutations and exception paths ---'
rg -n -C 4 --glob '*.py' \
'(Pg8000DriverTlsPolicyError|_verified_remote_ssl_context|_is_explicit_loopback_host|ssl_context|create_default_context)' \
pg_llm_batch testsLength of output: 8887 🤖 web_search executed:
Length of output: 10116 🏁 Script executed: #!/bin/bash
set -euo pipefail
workdir="$(mktemp -d)"
trap 'rm -rf "$workdir"' EXIT
curl --fail --silent --show-error --location \
'https://files.pythonhosted.org/packages/source/p/pg8000/pg8000-1.31.5.tar.gz' \
--output "$workdir/pg8000-1.31.5.tar.gz"
printf '%s\n' '--- published source archive digest ---'
sha256sum "$workdir/pg8000-1.31.5.tar.gz"
tar --extract --gzip --file "$workdir/pg8000-1.31.5.tar.gz" \
--to-stdout '*/src/pg8000/core.py' > "$workdir/core.py"
printf '%s\n' '--- exact TLS socket wrapping context ---'
rg -n -C 12 'wrap_socket|ssl_context|server_hostname' "$workdir/core.py"Length of output: 5014
I found no blocking source-level issue in the two-file delta from
The hosted gates are not terminal. The coverage/lint/package check currently reports failure, but GitHub does not yet expose its log because the workflow remains in progress. I cannot treat that check as verified until it completes.
|
|
@coderabbitai review Please review exact current head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base='b68a975becc93841dbfea818beccb8119f2f6944'
head='5588dc9aed0ce028cf056691e7333c65565e72e7'
printf '%s\n' '--- exact head and ancestry ---'
printf 'HEAD=%s\n' "$(git rev-parse HEAD)"
git merge-base --is-ancestor "$base" "$head" && echo 'base_is_ancestor=true'
git log --oneline "$base..$head"
printf '%s\n' '--- bounded relative delta ---'
git diff --stat "$base" "$head"
git diff --name-status "$base" "$head"
git diff --no-ext-diff --unified=60 "$base" "$head" -- \
pg_llm_batch/pg8000_driver_adapter.py \
tests/test_pg8000_remote_tls_identity.py \
docs/adr/0023-pg8000-remote-tls-server-identity.md
printf '%s\n' '--- current source, tests, and ADR ---'
cat -n pg_llm_batch/pg8000_driver_adapter.py
cat -n tests/test_pg8000_remote_tls_identity.py
cat -n docs/adr/0023-pg8000-remote-tls-server-identity.md
printf '%s\n' '--- inherited pg8000 connection and service-file boundaries ---'
ast-grep outline pg_llm_batch/pg8000_candidate_driver_port.py --items all
rg -n -A160 -B15 '^\s*def connect\(' pg_llm_batch/pg8000_candidate_driver_port.py
ast-grep outline pg_llm_batch/pg8000_candidate_service_file.py --items all
rg -n -A160 -B15 '^\s*(def __call__|def resolve|class Pg8000CandidateServiceFileResolver)' \
pg_llm_batch/pg8000_candidate_service_file.py
printf '%s\n' '--- TLS and adapter construction paths ---'
rg -n -C 6 --glob '*.py' \
'(ssl_context|create_default_context|CERT_REQUIRED|check_hostname|Pg8000DriverAdapter\s*\(|load_pg8000_driver\s*\(|Pg8000CandidateDriverAdapter\s*\()' \
pg_llm_batch tests
printf '%s\n' '--- dependency version declarations ---'
rg -n -C 3 '(pg8000)' \
pyproject.toml poetry.lock requirements.txt requirements-*.txt setup.py setup.cfg Pipfile Pipfile.lock 2>/dev/null || true
printf '%s\n' '--- pull request state and checks ---'
gh pr view 342 --repo ContextualWisdomLab/pg-llm-batch \
--json headRefOid,isDraft,reviewDecision,statusCheckRollup \
--jq '{headRefOid,isDraft,reviewDecision,statusCheckRollup:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}'Length of output: 114966 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- host validation implementation ---'
sed -n '105,140p' pg_llm_batch/pg8000_candidate_driver_port.py
printf '%s\n' '--- terminal CI failure summaries ---'
for job in 103145589067 103145589140 103145589057 103145589050 103145589230 103145588842; do
printf '\n### job=%s\n' "$job"
gh run view 34561698843 \
--repo ContextualWisdomLab/pg-llm-batch \
--job "$job" --log-failed || true
done
printf '%s\n' '--- pg8000 1.31.5 TLS implementation ---'
workdir="$(mktemp -d)"
trap 'rm -rf "$workdir"' EXIT
curl --fail --silent --show-error --location \
'https://files.pythonhosted.org/packages/source/p/pg8000/pg8000-1.31.5.tar.gz' \
--output "$workdir/pg8000-1.31.5.tar.gz"
tar --extract --gzip --file "$workdir/pg8000-1.31.5.tar.gz" \
--to-stdout '*/src/pg8000/core.py' > "$workdir/core.py"
rg -n -C 14 '(ssl_context|wrap_socket|server_hostname)' "$workdir/core.py"
printf '%s\n' '--- current PR check state ---'
gh pr view 342 --repo ContextualWisdomLab/pg-llm-batch \
--json statusCheckRollup \
--jq '.statusCheckRollup[] | {name,status,conclusion,detailsUrl}'Length of output: 69627
I found one blocking issue.
I found no additional source-level downgrade or policy-bypass path.
The container build and PostgreSQL runtime-smoke job succeeded. The PR remains Draft and has no passing full unit-test or coverage result at this head. You are interacting with an AI system. |
|
@coderabbitai review Please review exact current head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base='b68a975becc93841dbfea818beccb8119f2f6944'
head='2f81c255931b95cc246b1d85276d0725bcb47e96'
repo='ContextualWisdomLab/pg-llm-batch'
printf '%s\n' '--- exact head and ancestry ---'
printf 'checked_out=%s expected=%s\n' "$(git rev-parse HEAD)" "$head"
git merge-base --is-ancestor "$base" "$head" && printf 'base_is_ancestor=true\n'
git log --oneline "$base..$head"
printf '%s\n' '--- parent-relative surface and diff ---'
git diff --stat "$base" "$head"
git diff --name-status "$base" "$head"
git diff --no-ext-diff --unified=50 "$base" "$head" -- \
pg_llm_batch/pg8000_driver_adapter.py \
tests/test_pg8000_remote_tls_identity.py \
docs/adr/0023-pg8000-remote-tls-server-identity.md
printf '%s\n' '--- current changed files ---'
for file in \
pg_llm_batch/pg8000_driver_adapter.py \
tests/test_pg8000_remote_tls_identity.py \
docs/adr/0023-pg8000-remote-tls-server-identity.md
do
printf '\n### %s\n' "$file"
cat -n "$file"
done
printf '%s\n' '--- inherited host, connection, and service-file boundaries ---'
ast-grep outline pg_llm_batch/pg8000_candidate_driver_port.py --items all
sed -n '100,150p' pg_llm_batch/pg8000_candidate_driver_port.py
rg -n -A150 -B15 '^\s*def connect\(' pg_llm_batch/pg8000_candidate_driver_port.py
ast-grep outline pg_llm_batch/pg8000_candidate_service_file.py --items all
rg -n -A150 -B15 '^\s*(def __call__|def resolve|class Pg8000CandidateServiceFileResolver)' \
pg_llm_batch/pg8000_candidate_service_file.py
printf '%s\n' '--- all production TLS/raw-connect and construction paths ---'
rg -n -C 6 --glob '*.py' \
'(ssl_context|create_default_context|CERT_REQUIRED|check_hostname|raw_connect|Pg8000DriverAdapter\s*\(|load_pg8000_driver\s*\(|Pg8000CandidateDriverAdapter\s*\()' \
pg_llm_batch tests
printf '%s\n' '--- pg8000 dependency declaration ---'
rg -n -C 3 'pg8000' pyproject.toml poetry.lock 2>/dev/null || true
printf '%s\n' '--- PR and stated workflow evidence ---'
gh pr view 342 --repo "$repo" \
--json headRefOid,isDraft,reviewDecision,statusCheckRollup \
--jq '{headRefOid,isDraft,reviewDecision,statusCheckRollup:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}'
for run in 34561827011 34561826891; do
printf '\n### workflow_run=%s\n' "$run"
gh run view "$run" --repo "$repo" \
--json headSha,status,conclusion,name,event,workflowName,jobs \
--jq '{headSha,status,conclusion,name,workflowName,jobs:[.jobs[]|{name,status,conclusion}]}'
doneLength of output: 124410
I found no blocking issue in the three-file delta from
I verified the stated exact-head workflows:
The existing runtime smoke is loopback candidate-adapter coverage. It is not evidence of successful or failing remote TLS negotiation, CA trust, hostname mismatch handling, or plaintext-downgrade refusal against a real PostgreSQL server. You are interacting with an AI system. |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head source review on 2f81c255931b95cc246b1d85276d0725bcb47e96: the three-file delta closes the source-level pg8000 remote downgrade gap without widening the loopback exception. For every validated non-loopback TCP host, the production adapter injects an SSLContext that must retain hostname checking and CERT_REQUIRED before raw-driver access; exact localhost / IPv4 127/8 / IPv6 ::1 remain the deliberate local-development exception. This matches pg8000 1.31.5's documented distinction between ssl_context=None (SSL attempt with plaintext fallback if the server refuses), True (minimum-check SSL), and a caller-supplied SSLContext.
I found no additional source-level defect in the current three-file delta. This review is deliberately COMMENT, not approval: the current hosted PostgreSQL smoke uses the candidate adapter over loopback/plain PostgreSQL, so it does not exercise this production remote-TLS seam. Keep #123 and this Draft open until realistic TLS-enabled PostgreSQL acceptance proves trusted CA + matching identity success, untrusted/wrong CA failure, hostname mismatch failure, no remote plaintext downgrade, bounded diagnostics, and preservation of deliberate injected-driver authority. Branch-local CI/Release Acceptance does not substitute for that evidence or for independent approval/protected integration.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
현재 real TLS matrix 자체는 유효하지만 trust-policy authority에 한 가지 누락이 있습니다. Python 3.14의 ssl.create_default_context()는 SSLKEYLOGFILE이 설정되어 있으면 key logging을 활성화하고, OpenSSL 기본 CA 경로는 SSL_CERT_FILE/SSL_CERT_DIR 환경 키를 가질 수 있습니다. 그런데 ADR 0023은 ambient TLS environment authority를 도입하지 않는다고 명시하면서, 현재 smoke는 오히려 SSL_CERT_FILE로 production create_default_context()의 trust anchor를 바꿉니다. 즉 구현/acceptance와 명시 계약이 불일치하고, 특히 SSLKEYLOGFILE은 package-created remote TLS 세션 키를 ambient process setting으로 기록할 수 있습니다. 이 lane 안에서 최소 repair가 필요합니다: 적어도 key-log authority는 connection 전에 fail closed하고, CA environment authority는 허용/금지 중 하나를 명시적으로 결정해 ADR·acceptance와 일치시켜야 합니다. Peer verification이나 strict X.509를 낮추는 수정은 허용하지 않습니다.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
c61a91a...에서 지적한 TLS trust/key-export authority finding은 현재 head에서 source·test·ADR로 수리됐습니다. SSLKEYLOGFILE은 더 이상 package-created PostgreSQL SSLContext의 key-log sink가 되지 않고, hostname verification/CERT_REQUIRED/strict X.509는 유지됩니다. 반면 load_default_certs()가 사용하는 host/platform/OpenSSL CA store는 deployment trust authority로 명시적으로 문서화했습니다. 현재 real PostgreSQL TLS matrix도 새 context factory로 Python 3.10/3.12/3.14에서 성공했습니다. 이 COMMENT는 finding repair 확인이지 approval이 아닙니다. Draft ancestry, independent current-head approval, protected-stack integration 및 immutable release evidence는 별도 gate로 남습니다.
|
@coderabbitai review Please review exact current head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='ContextualWisdomLab/pg-llm-batch'
base='b68a975becc93841dbfea818beccb8119f2f6944'
head='11355fc8a3bf6fecadcdf34c2acc21ada9cbb2f1'
printf '%s\n' '--- exact commit and topology ---'
printf 'checked_out=%s expected=%s\n' "$(git rev-parse HEAD)" "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'base_is_ancestor=true'
git diff --stat "$base" "$head"
git diff --name-status "$base" "$head"
git log --oneline "$base..$head"
printf '%s\n' '--- bounded delta ---'
git diff --no-ext-diff --unified=50 "$base" "$head" -- \
pg_llm_batch/pg8000_driver_adapter.py \
tests/test_pg8000_remote_tls_identity.py \
tests/smoke_pg8000_candidate_postgres.py \
docs/adr/0023-pg8000-remote-tls-server-identity.md
printf '%s\n' '--- changed source, tests, and ADR ---'
for file in \
pg_llm_batch/pg8000_driver_adapter.py \
tests/test_pg8000_remote_tls_identity.py \
tests/smoke_pg8000_candidate_postgres.py \
docs/adr/0023-pg8000-remote-tls-server-identity.md
do
printf '\n### %s\n' "$file"
cat -n "$file"
done
printf '%s\n' '--- inherited connection, selector, and service-file behavior ---'
ast-grep outline pg_llm_batch/pg8000_candidate_driver_port.py --items all
sed -n '100,150p' pg_llm_batch/pg8000_candidate_driver_port.py
rg -n -A160 -B15 '^\s*def connect\(' pg_llm_batch/pg8000_candidate_driver_port.py
ast-grep outline pg_llm_batch/pg8000_candidate_service_file.py --items all
rg -n -A160 -B15 '^\s*(def __call__|def resolve|class Pg8000CandidateServiceFileResolver)' \
pg_llm_batch/pg8000_candidate_service_file.py
printf '%s\n' '--- context creation, direct connection, and construction paths ---'
rg -n -C 6 --glob '*.py' \
'(SSLContext|PROTOCOL_TLS_CLIENT|VERIFY_X509|load_default_certs|SSLKEYLOGFILE|keylog_filename|ssl_context|raw_connect|Pg8000DriverAdapter\s*\(|load_pg8000_driver\s*\(|Pg8000CandidateDriverAdapter\s*\()' \
pg_llm_batch tests
printf '%s\n' '--- pg8000 version declarations ---'
rg -n -C 3 'pg8000' pyproject.toml poetry.lock 2>/dev/null || true
printf '%s\n' '--- pull request and exact workflow evidence ---'
gh pr view 342 --repo "$repo" \
--json headRefOid,isDraft,reviewDecision,statusCheckRollup \
--jq '{headRefOid,isDraft,reviewDecision,statusCheckRollup:[.statusCheckRollup[]|{name,status,conclusion,detailsUrl}]}'
for run in 34566379642 34566379668; do
printf '\n### workflow_run=%s\n' "$run"
gh run view "$run" --repo "$repo" \
--json headSha,status,conclusion,name,workflowName,jobs \
--jq '{headSha,status,conclusion,name,workflowName,jobs:[.jobs[]|{name,status,conclusion}]}'
doneLength of output: 200661 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='ContextualWisdomLab/pg-llm-batch'
run='34566379642'
printf '%s\n' '--- repository workflow definitions that invoke the pg8000 smoke ---'
fd -t f -e yml -e yaml .github . 2>/dev/null | while IFS= read -r file; do
if rg -n -C 10 'smoke_pg8000_candidate_postgres|PG8000_CANDIDATE_CONTAINER|PG8000_CANDIDATE_PASSWORD_FILE' "$file"; then
printf '\n### %s\n' "$file"
fi
done
printf '%s\n' '--- successful runtime-smoke job steps and log lines ---'
gh run view "$run" --repo "$repo" --job 103159287394 --json name,steps \
--jq '{name,steps:[.steps[]|{name,status,conclusion,number}]}'
gh run view "$run" --repo "$repo" --job 103159287394 --log \
| rg -n -C 4 '(smoke_pg8000_candidate_postgres|Python [0-9]+\.[0-9]+|pg8000|remote TLS|TLS)' || true
printf '%s\n' '--- pg8000 1.31.5 TLS socket wrapping ---'
workdir="$(mktemp -d)"
trap 'rm -rf "$workdir"' EXIT
curl --fail --silent --show-error --location \
'https://files.pythonhosted.org/packages/source/p/pg8000/pg8000-1.31.5.tar.gz' \
--output "$workdir/pg8000-1.31.5.tar.gz"
tar --extract --gzip --file "$workdir/pg8000-1.31.5.tar.gz" \
--to-stdout '*/src/pg8000/core.py' > "$workdir/core.py"
rg -n -C 16 '(ssl_context|wrap_socket|server_hostname)' "$workdir/core.py"Length of output: 1942 🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='ContextualWisdomLab/pg-llm-batch'
run='34566379642'
job='103159287394'
printf '%s\n' '--- workflow definitions ---'
fd --hidden --no-ignore -t f -e yml -e yaml . | while IFS= read -r file; do
if rg -q 'smoke_pg8000_candidate_postgres|PG8000_CANDIDATE_CONTAINER|PG8000_CANDIDATE_PASSWORD_FILE' "$file"; then
printf '\n### %s\n' "$file"
rg -n -C 12 'smoke_pg8000_candidate_postgres|PG8000_CANDIDATE_CONTAINER|PG8000_CANDIDATE_PASSWORD_FILE|matrix:|python-version' "$file"
fi
done
printf '%s\n' '--- successful runtime-smoke job metadata and steps ---'
gh api "repos/$repo/actions/jobs/$job" \
--jq '{name,conclusion,started_at,completed_at,steps:[.steps[]|{number,name,status,conclusion}]}'
printf '%s\n' '--- successful runtime-smoke log evidence ---'
gh run view "$run" --repo "$repo" --job "$job" --log \
| rg -n -C 5 '(smoke_pg8000_candidate_postgres|python-version|Python [0-9]+\.[0-9]+|pg8000|remote TLS|TLS)' || true
printf '%s\n' '--- pg8000 1.31.5 TLS socket wrapping ---'
workdir="$(mktemp -d)"
trap 'rm -rf "$workdir"' EXIT
curl --fail --silent --show-error --location \
'https://files.pythonhosted.org/packages/source/p/pg8000/pg8000-1.31.5.tar.gz' \
--output "$workdir/pg8000-1.31.5.tar.gz"
tar --extract --gzip --file "$workdir/pg8000-1.31.5.tar.gz" \
--to-stdout '*/src/pg8000/core.py' > "$workdir/core.py"
rg -n -C 16 '(ssl_context|wrap_socket|server_hostname)' "$workdir/core.py"Length of output: 138735
I found no blocking issue in the four-file delta from
The authority split is correct.
The remote TLS boundary is fail-closed.
The real PostgreSQL smoke provides the required remote acceptance.
The inherited behavior remains intact.
I verified the exact-head acceptance evidence.
You are interacting with an AI system. |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review on be121f6300d939e4fe47a0d9950cd73821a95642: fresh review found that the prior remote-TLS diagnostic boundary normalized Python/OpenSSL ssl.SSLError but did not cover pg8000 1.31.5's DB-API InterfaceError path when a server refuses SSL. The previous real PostgreSQL negative-TLS helper also accepted any exception, so no-downgrade could pass without proving the package's fixed diagnostic boundary. Test-first 09a8e315... pinned the admitted server-refusal shape and unrelated-interface preservation; source child ec3676b4... added only the exact pg8000 refusal normalization; real-smoke child f93ac63e... now requires Pg8000DriverTlsPolicyError("PostgreSQL TLS policy is unavailable") with no cause for every negative TLS case. Exact 06e7723... then produced a real repository RED at CI 34587773445 because the new classifier's missing-InterfaceError-authority path broke the 100% production line-coverage gate; test-only be121f63... covered that branch without weakening the gate. Exact-current CI 34587906305 and Release Acceptance 34587906307 are both terminal success. This is deliberately COMMENT, not approval: #342/#323 remain Draft descendants, root #233 is not protected-integrated, the central compatibility prerequisite is unresolved, and no qualifying independent current-head APPROVED review exists.
|
@coderabbitai review |
✅ Action performedReview finished.
|
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head security review: the preceding content-free TLS diagnostic still retained sensitive lower-layer exceptions in Python's implicit __context__ because the replacement error was raised inside the active handler. Test-first a1d7baacd6a17d5d517cdfbae8a1e2cf037cf0fb required __cause__ is None and __context__ is None for trust-context construction, certificate-verification, and admitted pg8000 SSL-refusal failures; CI 34597113568 produced a real RED in Python 3.14 unit tests and the 100% coverage lane. Exact head 609ca9062ef16e14ce0b3d9ff25a5ce14efbd800 moves the fixed TLS-policy raise outside each sensitive handler while preserving unrelated DB-API failures. CI 34597262650 and Release Acceptance 34597262656 are terminal success, including the real PostgreSQL smoke. This is a COMMENT, not self-approval; Draft/integration/release gates remain.
|
@coderabbitai review |
✅ Action performedReview finished.
|
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head verification on 2dd438ffe9110072078190b512bc90fa011a538e: I tested a suspected trust-context confidentiality gap instead of accepting it by inspection. Test-first c2ccffdf23a34e213da0eed8f76af4bf01f06cb7 injects ssl.SSLError("secret default CA loader detail") from _new_remote_ssl_context() and requires the existing fixed TLS-policy error with no cause/context and no raw-driver call. The predecessor production code already passed because Python ssl.SSLError is an OSError subclass, so this is a falsified finding rather than RED evidence. Intermediate 582b3b36... made that subtype redundant in the catch tuple; ordinary child 2dd438ff... removes the non-causal source edit and restores the production adapter blob exactly to predecessor 609ca906..., retaining only the focused regression as net delta. Release Acceptance 34608256698 is terminal success; CI 34608256734 is still running, with completed quality and Python 3.11–3.14 unit jobs GREEN at this review. This is deliberately COMMENT, not self-approval; current-head terminal CI, independent approval, parent/protected integration and immutable release remain separate gates.
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Terminal follow-up on exact 2dd438ffe9110072078190b512bc90fa011a538e: CI 34608256734 and Release Acceptance 34608256698 are both completed/success. The exact-current quality lane passed compile, Ruff, 100% production docstrings, 100% production line coverage, lock/package build, and supported Python unit lanes; the existing component/PostgreSQL acceptance also completed successfully. This confirms the retained SSL-construction regression without introducing a production-code delta relative to predecessor 609ca906.... The review finding remains classified as falsified rather than repaired: ssl.SSLError was already covered through OSError. This remains COMMENT, not self-approval; independent approval, #323/#233 protected ancestry, central compatibility, and immutable release are still separate gates.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Fresh independent tool review has now completed on exact current head This closes the current-head review request only; it is not a qualifying GitHub |
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Fresh exact-head governance follow-up on 2dd438ffe9110072078190b512bc90fa011a538e: CodeRabbit run 0d205aef-798d-4b56-91b2-711ed050967e reviewed the only net delta from 609ca906... through this exact head and reported no actionable comments with minimal merge risk. Repository CI 34608256734 and Release Acceptance 34608256698 remain terminal GREEN, and the inline-thread inventory is empty. This is evidence only, not self-approval: formal reviews remain COMMENT-only and no qualifying independent APPROVED exists.
The central prerequisite description is also narrower than older comments: .github#2040 has completed ordinary/non-force reconciliation to protected .github/main@cb0872c9a20d5584703dffacca65c096fc034c6c at 3b2de64c2c4c95c56d2f5099a480a0825304d038; its exact-head CodeQL run 34629071379 remains failed on the central producer/consumer settlement path. The pg TLS source lane must stay unchanged while that owner repair and the immutable contextual-orchestrator release boundary (contextual-orchestrator#1083) settle. Draft remains correct; no no-op commit, synthetic status, provider/model/timeout override, force update, bypass, or predecessor-evidence transfer is justified.
seonghobae
added
bug
ChatGPT Codex Connector
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Current-head verification on d59df8ff14f8d82ac01f866cb38889407634cc46: this is an ordinary two-parent, non-force restack of the prior TLS-security head 2dd438ffe9110072078190b512bc90fa011a538e onto current parent #323 34858ef2f96307273853901bef932ec2958e4931. The merge base is exactly current #323, and the effective relative surface remains the same four owned paths; #323's intervening root-document authority cleanup is preserved rather than copied back into this branch. Fresh exact-head CI 34754512826 and Release Acceptance 34754512825 are both terminal SUCCESS, and the inline-thread inventory is empty. This COMMENT verifies the structural repair and exact-head repository evidence; it is deliberately not self-approval. #342 remains Draft because parent #323/root #233 and central required-workflow/review prerequisites are not protected-integrated, and branch-local GREEN is not immutable-release authority.
1 participant
Security gap
Issue #123 is the canonical durable owner for package-created remote PostgreSQL transport encryption and authenticated server identity. Parent #323 selects pg8000 1.31.5 behind
PostgresDriverPort. This Draft remains the dedicated source/acceptance owner for the remote TLS identity boundary.Exact current topology — 2026-09-16
main:5913c4bad79d6bc29d7cc1c624abb7db2ea6a77cat the latest verified repository sweep;8da8b9de7b89754cb10091d5457c88e255bfc727;34999370347: SUCCESS;34999370213: SUCCESS;0c1056fcb2cc33bc6d6a69e4f8db33117809778f;8da8b9de7b89754cb10091d5457c88e255bfc727;pg_llm_batch/pg8000_driver_adapter.py,tests/test_pg8000_remote_tls_identity.py,tests/smoke_pg8000_candidate_postgres.py, and Proposed ADR 0023;35023766139: SUCCESS;35023766945: SUCCESS.The reconciliation is an ordinary two-parent commit with the former child
d59df8ff14f8d82ac01f866cb38889407634cc46as first parent and current #323 as the additional parent. The tree takes current #323 as authority for every path except the four TLS-owned paths. No force update or destructive rebase was used.Before the reconcile, the two child-modified paths that also existed in the parent (
pg8000_driver_adapter.pyand the permanent pg8000/PostgreSQL smoke) were re-read on the old merge base34858ef2...and current parent8da8b9de...; both were unchanged across those six parent commits. The other two TLS paths are child-owned additions. Complete branch/open-PR census found no competing dedicated pg8000 remote-TLS writer. This is why a bounded tree-overlay merge-forward was safe rather than a blind conflict resolution.The reconciled current head is repository-local GREEN. Fresh formal reviews remain COMMENT-only with no qualifying independent APPROVED review, and the current inline review-thread inventory is empty. #323 remains Draft and root #233 remains outside protected integration, so #342 remains Draft and is not merge-authorized.
Repaired TLS contract retained
The production adapter preserves
PostgresDriverPort, selector grammar, finite connect timeout, transaction/thread-affinity/JSONB/SQLSTATE behavior, explicit service-file authority, exact pg8000 artifact admission, and caller-injected alternate-driver authority.Every validated non-loopback TCP target requires authenticated TLS, strict X.509 plus hostname/IP verification, host/platform default CA trust, no ambient
SSLKEYLOGFILEsink, no remote plaintext downgrade, and content-free failure diagnostics. Python/OpenSSLssl.SSLErrorand only the exact admitted pg8000InterfaceError("Server refuses SSL")are normalized to the fixed TLS-policy error; unrelated DB-API failures stay native. The public normalized error retains neither sensitive lower-layer__cause__nor__context__.Real PostgreSQL acceptance requires trusted CA + matching identity success, unrelated-CA rejection, mismatched identity rejection, and TLS-disabled remote no-downgrade on supported Python versions. ADR 0023 remains Proposed until normal protected integration and immutable-release evidence exist.
Finding-verification lineage retained
A prior review suspected
_verified_remote_ssl_context()could leak an OpenSSLssl.SSLErrorraised while_new_remote_ssl_context()constructs host trust. Test-firstc2ccffdf23a34e213da0eed8f76af4bf01f06cb7proved the predecessor production already normalized it becausessl.SSLErroris anOSErrorsubclass. Intermediate582b3b36...made the redundant subtype explicit; ordinary child2dd438ff...removed that non-causal source edit while retaining the regression. This remains recorded as a falsified finding, not invented RED→GREEN.The broader TLS lineage also retains the actual repairs for ambient key-log authority, certificate/hostname diagnostics, the exact pg8000 server-refusal downgrade path, and lower-layer exception-context retention. Do not weaken those invariants during later parent reconciliation.
Predecessor evidence — historical only
Exact predecessor
d59df8ff14f8d82ac01f866cb38889407634cc46had terminal repository GREEN against predecessor #323: CI34754512826and Release Acceptance34754512825both succeeded. Earlier exact2dd438ffe...also had terminal CI/Release Acceptance GREEN. Those receipts remain useful history for the unchanged TLS semantic slice but were not transferred to current acceptance;0c1056fc...reacquired its own successful CI and Release Acceptance.Integration boundary
Repository-owned GREEN does not by itself authorize merge. There is still no qualifying independent current-head APPROVED review. #323 remains Draft and root #233 remains outside protected
mainwhile central compatibility/review prerequisites remain unresolved. Branch-local GREEN is not protected-main or immutable-release authority.Root #233 remains the protected integration prerequisite. Normal order remains central prerequisite repair → #233 normal merge → ordinary/non-force #323 reconciliation/integration with fresh exact evidence → #342 reconciliation onto the actual integrated/current parent if needed → fresh exact-head/current-base repository, TLS, security/model-backed, review/thread, package/SBOM/provenance/reproducibility evidence → normal #342 integration → root documentation convergence through #324 → protected-head immutable version/CHANGELOG/tag/package/license/vulnerability/SBOM/provenance/reproducibility/rollback release and post-publication identity verification.
No force push, destructive rebase, synthetic status, gate weakening, routine administrator bypass, predecessor-evidence transfer, source-neutral wake commit, blind rerun, protected-main direct write, or self-approval is used. Any further ancestry movement must reacquire current-head/current-base evidence.
Refs #123, #233, #242, #244, #289, #290, #316, #321, #322, #323, #324, #346.