Conversation
|
✅ 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 (16)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughOAuth login status now exposes the latest continuation hint. The GUI updates its displayed URL, device code, and instructions from polling responses, and hides callback paste while a device code is active. Management provider discovery now lists Meta Muse only for the ChangesOAuth login continuations
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant onAuth
participant loginState
participant getLoginStatus
participant useProvidersOAuth
onAuth->>loginState: store the latest hint
useProvidersOAuth->>getLoginStatus: poll login status
getLoginStatus->>useProvidersOAuth: return current hint fields
useProvidersOAuth->>useProvidersOAuth: replace displayed hint
Merge Risk: ⚪ Minimal · up to No merge-blocking issue was established for the OAuth continuation and discovery changes. The cancellation delay predates this PR. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to An active login’s device code and instructions are now available through status polling. The status route does not apply the same principal check as the login-start route, creating a possible cross-principal disclosure during an active flow. The response does not expose stored credentials, and the effective reach of the status route remains to be confirmed. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 11 files. (5 skipped: 5 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
⏳ DRAFT
What to do
This pull request was already a draft. Its draft status will be preserved after every issue above is resolved. |
리뷰 · 우선순위 58 / 80이 PR은 대시보드의 Meta Muse 로그인에서 버튼과 안내 문구를 고칩니다. 관리자 토큰으로 연 대시보드는 Meta Muse 로그인을 눌러도 서버가 거절했습니다. 화면의 위험 경고에 동의해도 마찬가지였습니다. 서버는 브라우저 세션으로 연 대시보드만 이 로그인을 시작하게 합니다. 이 PR은 그 문을 넓히지 않습니다. 통과할 수 없는 화면에서는 Meta Muse를 목록에서 빼서, 눌러도 실패하는 버튼을 없앱니다. 다른 로그인 제공자는 그대로 보입니다. 터미널의 안내 문구도 바뀝니다. Meta Muse는 처음에 짧은 코드와 확인 링크를 보여 줍니다. 그 단계가 실패하면 키를 붙여 넣는 안내로 바뀝니다. 예전 화면은 첫 안내만 붙잡고 있어서, 코드가 지난 뒤에도 콜백 주소를 붙여 넣으라는 칸이 남았습니다. 이제는 서버가 최신 안내만 기억합니다. 주소, 사람이 치는 코드, 설명 세 가지입니다. 대시보드는 약 2초마다 상태를 물어 그 안내로 화면을 바꿉니다. 새 안내에 코드가 없으면 이전 코드도 지웁니다. 코드가 보이는 동안에는 붙여넣기 칸을 숨기고, 수동 단계가 되면 다시 보여 줍니다. 로그인이 끝나거나 취소되면 안내도 지웁니다. 토큰 같은 비밀 값은 이 안내에 넣지 않습니다. src/server/management/oauth-account-routes.ts:189 - 목록에서 빼는 조건과 로그인을 막는 조건이 같습니다. 관리자 토큰 대시보드에서는 Meta Muse 버튼이 사라집니다. 로그인이 가능해지지는 않습니다. 이슈 #5877을 연 사람은 허브에 관리자 토큰으로 붙어 있었습니다. gui/src/components/login-url-block.tsx:123 - 짧은 코드가 있으면 붙여넣기 칸을 모든 제공자에서 숨깁니다. Meta Muse는 코드 단계와 키 붙여넣기 단계를 나눠 알려 주므로 이 규칙과 맞습니다. 코드와 붙여넣기가 한 화면에 같이 있어야 하는 로그인이 있으면 그 칸이 안 보입니다. src/oauth/index.ts:1856 - 서버가 안내를 바꾼 직후가 아니라, 다음 상태 조회 뒤에 화면이 바뀝니다. 주기는 약 2초입니다. 디바이스 로그인이 실패해 키 입력을 기다리는 동안, 화면에는 잠깐 옛 코드가 남을 수 있습니다. 메인테이너의 판단이 필요한 지점 #5877을 이 PR로 닫을지입니다. 허브의 관리자 토큰 사용자에게도 로그인을 열어 줄 계획이었다면, 버튼을 숨기는 것으로는 부족합니다. 경고 동의만으로는 권한을 주지 않겠다는 방침이면, 이슈는 "버튼을 없앴다"로 닫아도 됩니다. 상태 조회 PR 게이트는 UI 스크린샷을 기다리고 있습니다. 디바이스 코드 화면과, 코드가 사라진 뒤 붙여넣기 칸이 다시 나온 화면이 있으면 확인이 쉽습니다. 너의 추천 옛 안내를 지우고, 코드가 있는 동안 붙여넣기 칸을 숨기는 쪽은 이대로 가져가도 됩니다. 권한을 넓히지 않은 점도 맞습니다. #5877은 닫기 전에, 허브 관리자 토큰으로는 Meta Muse 로그인을 열지 않을 것인지 한 줄로 정하면 됩니다. 스크린샷을 보태고 CI만 보면 머지 판단에 충분합니다. 본문에 전체 테스트는 돌리지 않았다고 적혀 있습니다. 이 댓글은 grok-bot이 작성했습니다 |
…ards (batch 9D) (#5986) A second `ocx` instance now leaves the live proxy's shared client routing alone across startup, management requests, restart, and stop. The batch also makes Home-side Remote Link usable from the local dashboard, keeps OAuth device guidance current, and offers one-time pairing on an authenticated remote hub that lacks a GUI session. | PR | Change | Author | |---|---|---| | #5926 | Isolate sibling startup, shared writes, restart and stop from the live owner. | JUN (lidge-jun) | | #5928 | Admit the local Home dashboard to SSH host discovery/probe/apply, improve SSH resolution and bounded errors, and keep Child join paired-only. | JUN (lidge-jun) | | #5911 | Show Meta Muse OAuth only to eligible GUI sessions and replace stale device hints through login. | Ingwannu, with shared device-hint work credited to codingbo | | #5978 | Show the one-time pairing form on Remote Link for a remote hub without a GUI session. | RHODIZSECURITY | The owner's three original commits retain JUN authorship. Each contributor PR remains one attributed squash commit. The older device-hint variant (#5915) is outside this branch because #5911 covers the same login behavior; its shared implementation is credited to codingbo. Follow-up commits after independent review: | Commit | Result | |---|---| | `66967095d0` | Restrict the new pairing display path to hub runtimes; a non-loopback standalone keeps its local-session guidance. | | `fa182b9d2d` | Keep sibling-home roster updates available while skipping the shared Claude Desktop profile auto-apply, including after asynchronous discovery. | | `32d7893113` | Require a fresh, home-bound listener proof before an orphan stop can signal a discovered proxy. | | `dda805fbad` | Require a short-lived, one-use sibling restart handoff record bound to the prior sibling runtime and home; a port env alone cannot claim sibling status. | | `37f368cf20` | Keep the connected-client sibling recycle path valid when its runtime record has no server attestation secret; the same-home PID, ports and process-local mark still gate issuance. | | `a42fcac751` | Capture the connected sibling's one-use handoff before link recycle stops the listener and removes its runtime record; spawn with that captured environment. | | `d3cc7b80bb` | Remove the Kiro cooldown test's timestamp-order race exposed by macOS CI. | `dev` advanced during review. Merge commit `a4d9aff73e` brought in `177c647d9c` and kept both the SSH PATH and listener-before-supervisor Remote Link contracts. Merge commit `43d314287c` brings in current `dev` `93e5d5bea5` without changing the owner's commits. Their combined dashboard structure document exceeded its line budget; `1e93a8401b` reflows the existing OAuth paragraph from 603 to 600 lines without raising the cap. The Kiro timing correction also landed independently on `dev`; the merge keeps that current test. Screenshots from the built GUI (demo session and responses only):    Security re-review should focus on sibling start/stop and handoff (`src/codex/sibling-start.ts`, `src/codex/sibling-handoff.ts`, `src/client/runtime.ts`, `src/cli/index.ts`, `src/server/proxy-liveness.ts`), Desktop auto-apply (`src/server/management/agent-settings-routes.ts`), Remote Link admission and SSH (`src/server/management/link-routes.ts`, `src/link/ssh-argv.ts`, `src/link/ssh-runner.ts`), OAuth state and principal discovery (`src/oauth/index.ts`, `src/oauth/login-flow-state.ts`, `src/server/management/oauth-account-routes.ts`), and the hub-only pairing gate (`gui/src/App.tsx`). Independent reviewer sign-off remains required before merge. Co-authored-by: Ingwannu <ingwannu@users.noreply.github.com> Co-authored-by: codingbo <cnsdbo@163.com> Co-authored-by: RHODIZSECURITY <180237049+RHODIZSECURITY@users.noreply.github.com>
2 participants
Summary
gui-sessionprincipal that can actually start iturl,instructions,deviceCode) and replace stale device hints during status pollingCloses #5877.
Closes #5881.
Why
The admin-token dashboard advertised Meta Muse even though both POST boundaries correctly require a consent-bearing GUI session, guaranteeing a 403 after the user accepted the risk prompt. Separately, the start response could only carry the first OAuth hint, so a device flow that changed steps left the GUI showing stale instructions and an invalid callback-paste affordance.
Discovery now reflects the existing admission rule without weakening it. The existing polling lifecycle carries only human-facing continuation fields; credentials and provider response objects never enter the DTO.
Validation
All commands ran in a disposable
HOME,CODEX_HOME, andOPENCODEX_HOMEunder a user systemd scope. Focused tests were capped atCPUQuota=75%,MemoryMax=1536M,MemorySwapMax=0, andTasksMax=64; structure/privacy checks usedCPUQuota=50%andMemoryMax=512M.bun test tests/oauth/oauth-public-surface.test.ts— 23 passbun test ./gui/tests/add-codex-account-device-code.test.tsx ./gui/tests/add-provider-oauth-url-leak.test.tsx ./gui/tests/provider-auth-device-code-copy.test.tsx— 49 passbun run structure:check— passbun run privacy:scan— passgit diff --check— passNo full suite or build was run locally.
Review notes
Summary by CodeRabbit