security: require exact bytes for request stream accounting - #85
security: require exact bytes for request stream accounting#85seonghobae wants to merge 15 commits into
Conversation
📝 WalkthroughWalkthrough동기·비동기 outbound request stream이 정확한 내장 Changes요청 스트림 보안 강화
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review Review only PR #85 at exact current head |
|
@opencode-agent Review only PR #85 exact head |
|
I will verify the exact built-in
|
|
@opencode-agent Perform a read-only review of exact current head Independently verify that synchronous and asynchronous request-stream wrappers require exact built-in @coderabbitai review Review only exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base='10d0c51daf2ad278d66f43be479df8cf6b08ba6d'
head='d89a82ae0fcb9d440404a3608824190d313aab1a'
echo '== Commit identity and ancestry =='
git rev-parse HEAD
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
git merge-base --is-ancestor "$base" "$head" && echo 'base is an ancestor of head'
git diff --stat "$base" "$head"
git diff --name-status "$base" "$head"
echo '== Relevant changed-code symbols =='
changed_files="$(git diff --name-only "$base" "$head")"
printf '%s\n' "$changed_files" | while IFS= read -r f; do
case "$f" in
*.py)
echo "--- AST outline: $f"
ast-grep outline "$f" --items all || true
;;
esac
done
echo '== Exact-type checks, length calls, cleanup, and denial paths in changed Python files =='
printf '%s\n' "$changed_files" | while IFS= read -r f; do
case "$f" in
*.py)
echo "--- $f"
rg -n -C 5 -e 'type\s*\([^)]*\)\s+is\s+bytes' -e '\blen\s*\(' \
-e '\b(close|aclose)\s*\(' -e '\b(raise|from None)\b' "$f" || true
;;
esac
done
echo '== Complete changed-file patch =='
git diff --find-renames --find-copies --unified=80 "$base" "$head" -- $changed_files
echo '== PR `#85` metadata, head, checks, and reviews =='
gh pr view 85 --repo ContextualWisdomLab/EgressWeave \
--json number,state,baseRefOid,headRefOid,headRefName,isDraft,mergeStateStatus,reviews,statusCheckRollup,urlLength of output: 70436 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Wrapper construction and surrounding transport logic =='
rg -n -C 12 -e '_BoundedSyncRequestStream\s*\(' -e '_BoundedAsyncRequestStream\s*\(' src tests
echo '== Request safety, authority, framing, TLS, proxy, and timeout references =='
rg -n -C 4 \
-e '_enforce_declared_request_size\s*\(' \
-e 'Content-Length|content-length|transfer-encoding|framing' \
-e 'proxy|trust_env|follow_redirects' \
-e 'sni_hostname|TLS|ssl_context|verify' \
-e 'timeout|Timeout' \
-e '_iteration_started|single-consumption|single consumption' \
src tests
echo '== Existing request-body tests and their focused assertions =='
fd -t f -e py . tests | sort | while IFS= read -r f; do
if rg -q -e 'request_body_safety|request.bytes|max_request_bytes|Content-Length|_Bounded.*RequestStream' "$f"; then
echo "--- $f"
ast-grep outline "$f" --items all
rg -n -C 3 -e 'max_request_bytes|Content-Length|_Bounded.*RequestStream|EgressNotAllowedError' "$f"
fi
done
echo '== New regression test: exact exception provenance and malformed values =='
sed -n '1,220p' tests/test_request_stream_chunk_validation.py
echo '== Files modified by each commit in the PR-only range =='
git log --format='commit %H%n%s' --name-status \
10d0c51daf2ad278d66f43be479df8cf6b08ba6d..d89a82ae0fcb9d440404a3608824190d313aab1aLength of output: 135734 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Synchronous and asynchronous wrapper installation =='
rg -n -C 24 '_Bounded(Sync|Async)RequestStream\s*\(' \
src/egressweave/sync_transport.py src/egressweave/transport.py
echo '== Request-target validation order and pool dispatch methods =='
ast-grep outline src/egressweave/sync_transport.py --items all --match 'handle|verify|close'
ast-grep outline src/egressweave/transport.py --items all --match 'handle|verify|close'
rg -n -C 18 \
-e 'def handle_request\s*\(' -e 'async def handle_async_request\s*\(' \
-e '_verify_request_target\s*\(' -e '_build_safe_request_headers\s*\(' \
-e '_enforce_request_header_limits\s*\(' -e 'handle_request\s*\(' \
-e 'handle_async_request\s*\(' \
src/egressweave/sync_transport.py src/egressweave/transport.py
echo '== Arbitrary malformed-object and exception-provenance tests =='
sed -n '1,110p' tests/test_request_body_chunk_types.py
rg -n -C 4 -e '__cause__|__context__|from None|private request stream' \
tests/test_request_body_cleanup.py tests/test_request_stream_chunk_validation.py \
src/egressweave/request_body_safety.py
echo '== Current PR `#85` check summary and approvals =='
gh pr view 85 --repo ContextualWisdomLab/EgressWeave \
--json headRefOid,reviews,statusCheckRollup,mergeStateStatus \
--jq '{head: .headRefOid, mergeStateStatus, reviews, checks: [.statusCheckRollup[] | {name, workflowName, status, conclusion, detailsUrl}]}'Length of output: 52948 🤖 web_search executed:
Length of output: 17876
I found one security issue.
Other review results:
|
|
@coderabbitai review Review only exact current head |
Rate Limit Exceeded
|
|
@coderabbitai review Review exact current head |
|
@opencode-agent Perform a read-only exact-head review of |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base='10d0c51daf2ad278d66f43be479df8cf6b08ba6d'
head='d89a82ae0fcb9d440404a3608824190d313aab1a'
repo='ContextualWisdomLab/EgressWeave'
echo '== Exact commit identity =='
printf 'checked-out: '; git rev-parse HEAD
git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"
git merge-base --is-ancestor "$base" "$head"
printf 'base: '; git rev-parse "$base"
printf 'head: '; git rev-parse "$head"
echo '== Changed paths =='
git diff --name-status "$base" "$head"
echo '== Diff stat =='
git diff --stat "$base" "$head"
echo '== Changed Python outlines =='
git diff --name-only "$base" "$head" -- '*.py' | while IFS= read -r path; do
echo "--- $path"
ast-grep outline "$path" --items all || true
done
echo '== Request-body implementation: exact-type checks, accounting, cleanup, and raises =='
if test -f src/egressweave/request_body_safety.py; then
nl -ba src/egressweave/request_body_safety.py | sed -n '1,260p'
fi
echo '== Focused changed tests =='
git diff --name-only "$base" "$head" -- 'tests/*.py' | while IFS= read -r path; do
echo "--- $path"
nl -ba "$path" | sed -n '1,320p'
done
echo '== Documentation and CHANGELOG patch =='
git diff --unified=60 "$base" "$head" -- \
'CHANGELOG.md' '*.md' '*.rst' '*.txt'
echo '== Complete code patch for changed production Python =='
git diff --unified=80 "$base" "$head" -- 'src/**/*.py' 'src/*.py'
echo '== PR `#85` exact head, checks, and reviews =='
gh pr view 85 --repo "$repo" \
--json number,state,baseRefOid,headRefOid,mergeStateStatus,reviews,statusCheckRollup,url \
--jq '{number,state,base: .baseRefOid,head: .headRefOid,mergeStateStatus,reviews,checks: [.statusCheckRollup[] | {name,workflowName,status,conclusion,headSha,detailsUrl}]}'Length of output: 4429 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='10d0c51daf2ad278d66f43be479df8cf6b08ba6d'
head='d89a82ae0fcb9d440404a3608824190d313aab1a'
repo='ContextualWisdomLab/EgressWeave'
echo '== request_body_safety.py at exact head =='
sed -n '1,280p' src/egressweave/request_body_safety.py
echo '== Focused regression tests at exact head =='
sed -n '1,360p' tests/test_request_stream_chunk_validation.py
echo '== Exact production-file patch =='
git diff --unified=100 "$base" "$head" -- src/egressweave/request_body_safety.py
echo '== Exact test-file patch =='
git diff --unified=100 "$base" "$head" -- tests/test_request_stream_chunk_validation.py
echo '== CHANGELOG patch =='
git diff --unified=100 "$base" "$head" -- CHANGELOG.md
echo '== Documentation patch =='
git diff --unified=100 "$base" "$head" -- docs/research/request-body-resource-limits.md
echo '== Related existing behavior and test definitions =='
rg -n -C 8 \
-e 'class _Bounded(Sync|Async)RequestStream' \
-e 'def _enforce_declared_request_size' \
-e 'async def test_.*(request|stream|content)' \
-e 'def test_.*(request|stream|content)' \
-e 'Content-Length|content-length|single.?consumption|single.?use' \
src tests
echo '== Current PR metadata, checks, and reviews =='
gh pr view 85 --repo "$repo" \
--json number,state,baseRefOid,headRefOid,mergeStateStatus,reviews,statusCheckRollup,url \
--jq '{number,state,url,base: .baseRefOid,head: .headRefOid,mergeStateStatus,reviews,checks: [.statusCheckRollup[] | {name,workflowName,status,conclusion,detailsUrl}]}'Length of output: 194342
I found one High-severity issue.
Other results:
|
|
@coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base='10d0c51daf2ad278d66f43be479df8cf6b08ba6d'
head='d89a82ae0fcb9d440404a3608824190d313aab1a'
repo='ContextualWisdomLab/EgressWeave'
echo '== Commit identity and ancestry =='
printf 'checked-out: '; git rev-parse HEAD
printf 'base: '; git rev-parse "$base"
printf 'head: '; git rev-parse "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor: yes'
echo '== Changed paths and stat =='
git diff --name-status "$base" "$head"
git diff --stat "$base" "$head"
echo '== Production implementation at requested head =='
git show "$head:src/egressweave/request_body_safety.py" | nl -ba | sed -n '1,280p'
echo '== Production patch =='
git diff --unified=100 "$base" "$head" -- src/egressweave/request_body_safety.py
echo '== Focused test outlines and patch =='
for path in \
tests/test_request_stream_chunk_validation.py \
tests/test_request_body_cleanup.py \
tests/test_request_body_chunk_types.py
do
if git cat-file -e "$head:$path" 2>/dev/null; then
echo "--- $path"
git show "$head:$path" | nl -ba | sed -n '1,420p'
fi
done
git diff --unified=100 "$base" "$head" -- tests
echo '== Documentation and CHANGELOG patches =='
git diff --unified=100 "$base" "$head" -- CHANGELOG.md docs
echo '== Wrapper installation and related existing invariants =='
rg -n -C 8 \
-e '_BoundedSyncRequestStream\s*\(' \
-e '_BoundedAsyncRequestStream\s*\(' \
-e '_enforce_declared_request_size\s*\(' \
-e 'Content-Length|content-length' \
-e '_iteration_started' \
src tests
echo '== PR metadata and fresh check evidence =='
gh pr view 85 --repo "$repo" \
--json number,state,baseRefOid,headRefOid,mergeStateStatus,reviews,statusCheckRollup,url \
--jq '{number,state,url,base: .baseRefOid,head: .headRefOid,mergeStateStatus,reviews,checks: [.statusCheckRollup[] | {name,workflowName,status,conclusion,detailsUrl}]}'Length of output: 2986 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='10d0c51daf2ad278d66f43be479df8cf6b08ba6d'
head='d89a82ae0fcb9d440404a3608824190d313aab1a'
repo='ContextualWisdomLab/EgressWeave'
echo '== Production implementation at requested head =='
git show "$head:src/egressweave/request_body_safety.py" | cat -n | sed -n '1,280p'
echo '== Production patch =='
git diff --unified=100 "$base" "$head" -- src/egressweave/request_body_safety.py
echo '== Focused test file at requested head =='
git show "$head:tests/test_request_stream_chunk_validation.py" | cat -n | sed -n '1,420p'
echo '== All test changes =='
git diff --unified=100 "$base" "$head" -- tests
echo '== Documentation and CHANGELOG changes =='
git diff --unified=100 "$base" "$head" -- CHANGELOG.md docs/research/request-body-resource-limits.md
echo '== Wrapper installation and related invariants =='
rg -n -C 10 \
-e '_BoundedSyncRequestStream\s*\(' \
-e '_BoundedAsyncRequestStream\s*\(' \
-e '_enforce_declared_request_size\s*\(' \
-e 'Content-Length|content-length' \
-e '_iteration_started' \
src tests
echo '== PR metadata and status checks =='
gh pr view 85 --repo "$repo" \
--json number,state,baseRefOid,headRefOid,mergeStateStatus,reviews,statusCheckRollup,url \
--jq '{number,state,url,base: .baseRefOid,head: .headRefOid,mergeStateStatus,reviews,checks: [.statusCheckRollup[] | {name,workflowName,status,conclusion,detailsUrl}]}'Length of output: 150804
I found one High-severity issue.
Other results:
|
|
@cwl-noema-review @opencode-agent Please perform a read-only formal review of PR #85 at exact head |
|
@opencode-agent Apply a bounded test-first fix only to exact current head A current valid asynchronous cleanup gap remains in Use strict RED → GREEN:
Run |
|
@coderabbitai review |
Rate Limit Exceeded
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@src/egressweave/request_body_safety.py`:
- Around line 130-143: Update _close_async_request_after_policy_denial so
invoking stream.aclose() and awaiting asyncio.gather are both inside the
protected cleanup path; suppress synchronous exceptions and non-awaitable return
failures rather than replacing EgressNotAllowedError, while re-raising only
cancellation directed at the awaiting consumer. Add a regression test covering
aclose returning a non-awaitable value and verifying the policy-denial error
remains observable.
🪄 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: a26e7bfe-f7da-4c63-baf5-eef033c84262
📒 Files selected for processing (4)
CHANGELOG.mddocs/research/request-body-resource-limits.mdsrc/egressweave/request_body_safety.pytests/test_request_stream_chunk_validation.py
|
@coderabbitai review |
Rate Limit Exceeded
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
@opencode-agent Repair only the test-fixture lint blocker on exact current head Hosted RED run Do not modify production code yet. Run Ruff and |
|
@coderabbitai review exact current head |
|
@opencode-agent Read-only review exact current head |
|
@cwl-noema-review Read-only review exact current head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base='10d0c51daf2ad278d66f43be479df8cf6b08ba6d'
head='270b5ce84b93cdc10e417990e86da0622c9ff16f'
repo='ContextualWisdomLab/EgressWeave'
echo '== Commit identity and ancestry =='
printf 'checked-out: '; git rev-parse HEAD
printf 'base: '; git rev-parse "$base"
printf 'head: '; git rev-parse "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor: yes'
echo '== Exact changed paths =='
git diff --name-status "$base" "$head"
git diff --stat "$base" "$head"
echo '== Request-body safety structure =='
ast-grep outline src/egressweave/request_body_safety.py --items all
echo '== Current request-body implementation =='
git show "$head:src/egressweave/request_body_safety.py" | nl -ba | sed -n '1,300p'
echo '== Focused regression test structure and content =='
ast-grep outline tests/test_request_stream_chunk_validation.py --items all
git show "$head:tests/test_request_stream_chunk_validation.py" | nl -ba | sed -n '1,500p'
echo '== Exact production and test patches =='
git diff --unified=100 "$base" "$head" -- \
src/egressweave/request_body_safety.py \
tests/test_request_stream_chunk_validation.py
echo '== Documentation and changelog patches =='
git diff --unified=100 "$base" "$head" -- \
docs/research/request-body-resource-limits.md \
CHANGELOG.md
echo '== Wrapper installation and preserved request invariants =='
rg -n -C 10 \
-e '_BoundedSyncRequestStream\s*\(' \
-e '_BoundedAsyncRequestStream\s*\(' \
-e '_enforce_declared_request_size\s*\(' \
-e 'Content-Length|content-length' \
-e '_iteration_started' \
src tests
echo '== PR current head, check rollup, and reviews =='
gh pr view 85 --repo "$repo" \
--json number,state,baseRefOid,headRefOid,mergeStateStatus,reviews,statusCheckRollup,url \
--jq '{number,state,url,base: .baseRefOid,head: .headRefOid,mergeStateStatus,reviews,checks: [.statusCheckRollup[] | {name,workflowName,status,conclusion,detailsUrl}]}'Length of output: 3647 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='10d0c51daf2ad278d66f43be479df8cf6b08ba6d'
head='270b5ce84b93cdc10e417990e86da0622c9ff16f'
repo='ContextualWisdomLab/EgressWeave'
echo '== Current request-body implementation =='
git show "$head:src/egressweave/request_body_safety.py" | cat -n | sed -n '1,300p'
echo '== Focused regression test file =='
git show "$head:tests/test_request_stream_chunk_validation.py" | cat -n | sed -n '1,520p'
echo '== Exact production patch =='
git diff --unified=100 "$base" "$head" -- src/egressweave/request_body_safety.py
echo '== Exact focused-test patch =='
git diff --unified=100 "$base" "$head" -- tests/test_request_stream_chunk_validation.py
echo '== Documentation and changelog patches =='
git diff --unified=100 "$base" "$head" -- \
docs/research/request-body-resource-limits.md \
CHANGELOG.md
echo '== Wrapper installation and preserved request invariants =='
rg -n -C 10 \
-e '_BoundedSyncRequestStream\s*\(' \
-e '_BoundedAsyncRequestStream\s*\(' \
-e '_enforce_declared_request_size\s*\(' \
-e 'Content-Length|content-length' \
-e '_iteration_started' \
src tests
echo '== PR current head, check rollup, and reviews =='
gh pr view 85 --repo "$repo" \
--json number,state,baseRefOid,headRefOid,mergeStateStatus,reviews,statusCheckRollup,url \
--jq '{number,state,url,base: .baseRefOid,head: .headRefOid,mergeStateStatus,reviews,checks: [.statusCheckRollup[] | {name,workflowName,status,conclusion,detailsUrl}]}'Length of output: 169678
I found one actionable security issue.
Other results:
|
|
@opencode-agent @cwl-noema-review Perform a read-only formal review of PR #85 at exact head |
|
@opencode-agent @cwl-noema-review Perform a read-only formal review of PR #85 at exact unchanged head |
|
@coderabbitai review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base='10d0c51daf2ad278d66f43be479df8cf6b08ba6d'
head='270b5ce84b93cdc10e417990e86da0622c9ff16f'
repo='ContextualWisdomLab/EgressWeave'
echo '== Exact commit identity and ancestry =='
printf 'checked-out: '; git rev-parse HEAD
printf 'base: '; git rev-parse "$base"
printf 'head: '; git rev-parse "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor: yes'
echo '== Exact changed paths =='
git diff --name-status "$base" "$head"
git diff --stat "$base" "$head"
echo '== Request-body implementation at exact head =='
git show "$head:src/egressweave/request_body_safety.py" | cat -n | sed -n '1,330p'
echo '== Focused regression tests at exact head =='
git show "$head:tests/test_request_stream_chunk_validation.py" | cat -n | sed -n '1,560p'
echo '== Exact source and test diffs =='
git diff --unified=100 "$base" "$head" -- \
src/egressweave/request_body_safety.py \
tests/test_request_stream_chunk_validation.py
echo '== Documentation and changelog diffs =='
git diff --unified=80 "$base" "$head" -- \
docs/research/request-body-resource-limits.md \
CHANGELOG.md
echo '== Wrapper construction and declared-length enforcement =='
rg -n -C 10 \
-e '_BoundedSyncRequestStream\s*\(' \
-e '_BoundedAsyncRequestStream\s*\(' \
-e 'def _enforce_declared_request_size\s*\(' \
-e 'Content-Length|content-length' \
-e '_iteration_started' \
src tests
echo '== Current PR metadata, reviews, and checks =='
gh pr view 85 --repo "$repo" \
--json number,state,baseRefOid,headRefOid,mergeStateStatus,reviews,statusCheckRollup,url \
--jq '{number,state,url,base: .baseRefOid,head: .headRefOid,mergeStateStatus,reviews,checks: [.statusCheckRollup[] | {name,workflowName,status,conclusion,detailsUrl}]}'Length of output: 164811
I found one current-head security issue.
Other exact-head results:
The documentation and changelog cleanup-masking claims remain inaccurate until direct You are interacting with an AI system. |
|
@coderabbitai approve Formal exact-head approval request for PR #85 at |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
base='10d0c51daf2ad278d66f43be479df8cf6b08ba6d'
head='270b5ce84b93cdc10e417990e86da0622c9ff16f'
repo='ContextualWisdomLab/EgressWeave'
echo '== Commit identity and ancestry =='
printf 'checked-out: '; git rev-parse HEAD
printf 'base: '; git rev-parse "$base"
printf 'head: '; git rev-parse "$head"
git merge-base --is-ancestor "$base" "$head" && echo 'base-is-ancestor: yes'
echo '== Changed paths =='
git diff --name-status "$base" "$head"
echo '== Request-body safety implementation at exact head =='
git show "$head:src/egressweave/request_body_safety.py" | nl -ba | sed -n '1,280p'
echo '== Focused cleanup regressions at exact head =='
git show "$head:tests/test_request_stream_chunk_validation.py" | nl -ba | sed -n '1,560p'
echo '== Exact source and test diff =='
git diff --unified=100 "$base" "$head" -- \
src/egressweave/request_body_safety.py \
tests/test_request_stream_chunk_validation.py
echo '== Documentation and changelog diff =='
git diff --unified=80 "$base" "$head" -- \
docs/research/request-body-resource-limits.md \
CHANGELOG.md
echo '== PR head, checks, reviews, and review threads =='
gh pr view 85 --repo "$repo" \
--json number,state,baseRefOid,headRefOid,mergeStateStatus,reviews,statusCheckRollup,url \
--jq '{number,state,url,base: .baseRefOid,head: .headRefOid,mergeStateStatus,reviews,checks: [.statusCheckRollup[] | {name,workflowName,status,conclusion,detailsUrl}]}'
gh api graphql -F owner='ContextualWisdomLab' -F name='EgressWeave' -F number=85 -f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes { isResolved isOutdated comments(first: 1) { nodes { author { login } body path } } }
}
}
}
}' --jq '[.data.repository.pullRequest.reviewThreads.nodes[] |
select((.isResolved | not) and (.isOutdated | not)) |
{author: .comments.nodes[0].author.login, path: .comments.nodes[0].path, body: .comments.nodes[0].body}]'Length of output: 2835 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='10d0c51daf2ad278d66f43be479df8cf6b08ba6d'
head='270b5ce84b93cdc10e417990e86da0622c9ff16f'
repo='ContextualWisdomLab/EgressWeave'
echo '== Request-body safety implementation at exact head =='
git show "$head:src/egressweave/request_body_safety.py" | cat -n | sed -n '1,280p'
echo '== Focused cleanup regressions at exact head =='
git show "$head:tests/test_request_stream_chunk_validation.py" | cat -n | sed -n '1,560p'
echo '== Cleanup call sites and exact-type checks =='
git show "$head:src/egressweave/request_body_safety.py" |
grep -n -E 'type\(chunk\) is bytes|close\(|aclose\(|BaseException|CancelledError|gather|from None|EgressNotAllowedError' || true
echo '== Exact source and test diff =='
git diff --unified=100 "$base" "$head" -- \
src/egressweave/request_body_safety.py \
tests/test_request_stream_chunk_validation.py
echo '== Documentation and changelog diff =='
git diff --unified=80 "$base" "$head" -- \
docs/research/request-body-resource-limits.md \
CHANGELOG.md
echo '== PR head, checks, reviews =='
gh pr view 85 --repo "$repo" \
--json number,state,baseRefOid,headRefOid,mergeStateStatus,reviews,statusCheckRollup,url \
--jq '{number,state,url,base: .baseRefOid,head: .headRefOid,mergeStateStatus,reviews,checks: [.statusCheckRollup[] | {name,workflowName,status,conclusion,detailsUrl}]}'
echo '== Unresolved, current review threads =='
gh api graphql -F owner='ContextualWisdomLab' -F name='EgressWeave' -F number=85 -f query='
query($owner: String!, $name: String!, $number: Int!) {
repository(owner: $owner, name: $name) {
pullRequest(number: $number) {
reviewThreads(first: 100) {
nodes { isResolved isOutdated comments(first: 1) { nodes { author { login } body path } } }
}
}
}
}' --jq '[.data.repository.pullRequest.reviewThreads.nodes[] |
select((.isResolved | not) and (.isOutdated | not)) |
{author: .comments.nodes[0].author.login, path: .comments.nodes[0].path, body: .comments.nodes[0].body}]'Length of output: 86416
One current-head security finding remains.
The exact head has the requested four-file scope. The exact-built-in- The central dependency-review repair and a fresh post-repair Security Scan remain independent merge prerequisites. ✏️ Learnings added
You are interacting with an AI system. |
|
@opencode-agent Apply one bounded test-first security fix only if PR #85 still has exact head Current exact-head CodeRabbit review found a valid remaining trust-boundary defect: policy-denial cleanup suppresses ordinary Use strict RED→GREEN and keep the existing four-path ownership boundary:
Do not touch |
|
Exact-head review disposition for |
1 participant
Replacement and exact-tree binding
Supersedes closed draft #80 without transferring review or approval. Protected-main base and live protected-main tip remain
10d0c51daf2ad278d66f43be479df8cf6b08ba6d; current exact head is8a1243feafd12f0faf67cc9fa12d53bd27181a14.Security boundary
Outbound request-body accounting accepts only exact built-in
byteschunks before invoking length behavior or exposing data downstream. It preserves cumulative byte budgets, exact declaredContent-Length, single-consumption semantics, exact authority/framing, TLS identity, proxy isolation, and timeout behavior.When policy denial has already been decided, dependency-injected source cleanup is a narrow untrusted boundary:
BaseExceptionsubclasses are consumed only inside policy-denial cleanup/setup so they cannot replace or become provenance for the generic denial;KeyboardInterrupt,SystemExit, andGeneratorExitraised directly by cleanup are explicitly re-raised;await cleanupis outside theBaseExceptioncatch;The caller receives a fresh
EgressNotAllowedError("egress URL is not allowed")with neither private context nor cause. No workflow, dependency, credential, permission, authority/TLS/proxy policy, public API, version, release/tag/publication path, or protected-ref behavior changes.TDD evidence
The branch preserves its earlier exact-byte and async-cleanup RED/GREEN history. The latest direct-
BaseExceptionrepair is independently test-first:a0418869890fd146462237f9c347419f0cb70484adds sync and async hostile cleanup sources that raise a custom directBaseException, plus guards requiringKeyboardInterrupt,SystemExit, andGeneratorExitto propagate;31270939632passed Ruff and package acceptance but failed the two new masking contracts across Python 3.10–3.13; Python 3.13 reported2 failed, 763 passed, with the custom child error escaping at the synchronousself._stream.close()and asynchronousstream.aclose()boundaries;8a1243feafd12f0faf67cc9fa12d53bd27181a14introduces a dedicated synchronous policy-denial cleanup helper and narrows both sync/asyncBaseExceptioncatches to direct untrusted cleanup/setup, explicitly preserving interpreter control flow and keeping the outer async await outside the catch;31271043659succeeds on Python 3.10, 3.11, 3.12, and 3.13 with package acceptance, Ruff, product-guard, and compileall all successful; Python 3.13 reports765 passed,1,637/1,637production statements and554/554branches at 100%;request_body_safety.pyis97/97statements and32/32branches;31271043648succeeds on the same exact head.The two same-head GitHub Code Quality suggestions to replace the narrow
BaseExceptioncatches withExceptionwere disproven by the preceding exact RED: doing so deterministically restores the reproduced direct-child exception leak. They were documented as false positives and resolved only after the GREEN evidence proved both masking and control-flow preservation.Standards and documentation
docs/research/request-body-resource-limits.mdrecords RFC 9110, RFC 9112, CWE-400, CWE-444, Python asyncio semantics, and the HTTPX transport interface with APA 7th references.[Unreleased]records the same security boundary without a version bump.Security-scan state
Security Scan run
31271043638is aggregate green for executed jobs on exact head8a1243feafd12f0faf67cc9fa12d53bd27181a14, including OSV, Trivy, and Scorecard. It is not complete dependency-review assurance: thedependency-reviewjob concluded success while its actualDependency reviewaction step wasskippedafter only the support probe ran.The separately governed control-plane sequence remains
ContextualWisdomLab/.github#813followed by refreshed/integrated.github#799. This EgressWeave loop does not mutate that repository. After the fail-closed central repair reaches protected main, this unchanged exact head requires a fresh organization Security Scan whose actual dependency-review action executes and succeeds.Scope
Changed paths are exactly:
src/egressweave/request_body_safety.pytests/test_request_stream_chunk_validation.pytests/test_request_cleanup_base_exception.pydocs/research/request-body-resource-limits.mdCHANGELOG.mdRemaining merge gates
Keep this PR Draft. Do not merge or enable auto-merge until the central dependency-review repair is protected-main integrated, a fresh same-head dependency review actually executes successfully, exact-current-head automated review is complete with zero unresolved valid threads, a qualifying independent non-author formal
APPROVEDreview exists, and branch protection/rulesets/every required check pass.Queued, pending, skipped-required, cancelled, absent, neutral-required, predecessor-head, synthetic-merge, fail-open, rate-limited, or failed evidence is not acceptance. Do not release, publish, rebase, retarget, or weaken the trust boundary.