Conversation
|
@lidge-jun security-boundary review requested on exact head |
|
✅ Deterministic PR hygiene checks passed. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (5)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe Child link relay now removes caller-supplied Azure ChangesLink relay credential boundary
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The relay keeps the named provider credentials on the Child while preserving the link key and ordinary request metadata. No identified issue prevents merging, subject to normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change reduces credential exposure for linked machines without expanding relay access. The remaining uncertainty is whether every deployment path uses the inspected relay and whether future provider credential names will receive the same protection. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. (3 skipped: 3 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 |
|
@lidge-jun exact security-fix head |
리뷰 · 우선순위 42 / 80이 PR은 Child가 Home으로 요청을 넘기기 전에, 호출자가 붙인 Azure 키와 Google 키를 빼 버립니다. Child와 Home은 서로 다른 컴퓨터입니다. Child는 링크 열쇠만 보여 주고, Home은 자기 계정으로 답을 합니다. 예전 목록은 지금은 그 두 이름이 목록에 있습니다. 비교 전에 이름을 소문자로 바꾸므로 라인 - 메인테이너의 판단이 필요한 지점 다음 제공자가 다른 헤더 이름으로 키를 보내면, 그 이름은 아직 통과합니다. ADR-6032는 이름에 주소의 물음표 뒤도 터널로 넘어갑니다. 너의 추천 머지하세요. #6032를 닫으면 됩니다. 베이스는 이 댓글은 grok-bot이 작성했습니다 |
|
Landed on |
Carried from lidge-jun#6034 into merge train round 3. Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com>
Summary
api-keyand Googlex-goog-api-keyalongside the existing caller credential headers before Child → Home forwardingidempotency-key, framing rules, and/v1/usagededicated-key behaviorCloses #6032.
Security boundary
The Home is a distinct credential owner. A linked Child replaces caller credentials with the link key and uses the Home's accounts. Before this patch, built-in Azure/Google credential forms were not in the explicit denylist and survived forwarding. This is a conditional paired-machine disclosure, not an unauthenticated Internet path.
The fix stays explicit rather than using a broad
*key*heuristic, which could incorrectly remove protocol metadata such asidempotency-key. Custom authentication header names still require deliberate review when introduced.Validation
All local commands ran one at a time in transient user units with
CPUQuota=75%,MemoryHigh=1G,MemoryMax=1536M, swap disabled, and disposable HOME/CODEX_HOME/OPENCODEX_HOME/TMPDIR; lightweight gates used 50% CPU and 512 MiB max.bun test tests/clients/client-link-relay.test.ts— 23 pass, 0 failbun run structure:check— passbun run privacy:scan— passbun scripts/file-size-ratchet.ts— passgit diff --check— passNo full suite/build, live service restart, production-home command, or credential was used.
Summary by CodeRabbit
/v1/usageforwards only its dedicated OpenCodeX key.