fix(egress): require headerless anonymous GitHub meta requests - #508
Conversation
📝 WalkthroughWalkthroughGitHub API 이그레스 검증, GitHub App 설치 ID discovery, 패치 검증 이미지 빌드, CI 진단 및 보안 지침이 변경되었습니다. 관련 테스트는 새 allowlist, abort, 설치 ID, dependency context 및 workflow 계약을 검증합니다. ChangesGitHub API 이그레스 정책
GitHub App 설치 ID 확인
패치 검증 이미지와 CI
Estimated code review effort: 5 (Critical) | ~90 minutes Merge Risk: 🔵 Low · up to The egress policy currently allows anonymous metadata requests carrying headers, weakening the intended fail-closed request contract. This is a bounded security-hardening risk that should have explicit owner follow-up before or with merge; the other noted items do not introduce production impact. Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 31 files. (4 skipped: 4 unsupported.)
✨ Finishing Touches 💡 1📝 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
seonghobae
changed the title
1 participant
Scope
Follow-up hardening after #500 integrated into protected
main. This PR now contains only the post-#500 successor delta: anonymousGET /metamust be bodyless and headerless, the cancellation regression verifies rejection before delayed body release, and the patch-validator contract binds the Node source URL and reviewed digest in the samedownload_exactinvocation.Current authority
main@d53897f28abb0aca6d3dd2c0295f0c4f4e81f076(GitHub-signed squash integration of fix(egress): bound anonymous GitHub API authority #500);d51ef8ac0cf87fda183c7680c2f2b017e496d10b;ef0d57ed25890479dc26c92c5ba0a49114d4c1f6, exactly 5 commits ahead ofd51ef8...with four changed files;738aa68ad7865ff2e4b9f57f175fa2201e66395c.Protected
main@d53897...has the same tree as predecessord51ef8.... To preserve the successor bytes without destructive rebase or force push, current head738aa68...is a two-parent convergence commit whose first parent isef0d57..., whose second parent is protectedmain@d53897..., and whose tree is exactly the successor tree. Fresh comparison now resolves protected main as the merge base (behind=0).Review lineage
The three concrete CodeRabbit findings on this successor were addressed before convergence:
/metarequests now requirehasNoHeaders(headers)and have a regression proving even a reviewedAcceptheader is rejected;$NODE_SOURCE_SHA256in onedownload_exactcontract.Observed review threads are resolved/outdated. Devin's latest predecessor-head review reported 0 new potential issues.
Exact-head evidence boundary
All checks from
ef0d57...and earlier are predecessor evidence only. The ref convergence regenerated Application, reviewer-ci, required central Security Scan, and dedicated patch-validator image/runtime/SBOM/provenance workflows on exact head738aa68...; pending/queued/in-progress evidence remains non-passing until terminal success on that unchanged head.Do not merge if the exact head or protected base moves, any applicable gate is not terminal-clean, or a valid unresolved review finding appears. Never transfer predecessor evidence, bypass the ruleset, force-push, or self-approve.