Skip to content

fix(meta-muse): consolidate login admission and bounded response handling - #5591

Closed
luvs01 wants to merge 6 commits into
devfrom
stack/meta-login-boundaries
Closed

luvs01 wants to merge 6 commits into
devfrom
stack/meta-login-boundaries

Conversation

@luvs01

@luvs01 luvs01 commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator

Summary

Consolidate #5234 and #5432 around the same Meta Muse credential-acquisition path. Management login requires the server-resolved dashboard session principal before importing a local credential or starting a device flow, and device authorization/token/mint responses are bounded and released on failure.

  • Reject raw management tokens and forged dashboard headers with oauth_consent_required before credential acquisition. The same admission applies to ordinary login, add-account and reauthentication; direct CLI and other OAuth-provider policies are unchanged.
  • Bound authorization, token-poll and key-mint JSON bodies to 64 KiB under the existing request deadline, validate UTF-8 and retain token-size limits. Cancel unparsed error bodies, including the previously missed mint-429 branch, without reflecting their contents.
  • Preserve rate-limit status, Retry-After handling and generic public OAuth error mapping. Synthetic integration coverage connects management admission to the actual bounded device-response reader.
  • Document macOS local-import preference and device/manual fallback accurately. A dashboard session is an admission requirement; this change does not introduce a per-checkbox consent receipt.

Verification

  • Exact head: e1a9eb1d14d8b39acfc34b855dc876c979ae0780, base dev a4bdc03054d4a449c7762eec9853f04557a06fb4. The dev refresh includes upstream release/CI and roster-test corrections. All eight Meta integration files and proxy runtime sources under src/ remain byte-identical to the tested c3a2f379cefba4aad998492a619b058003dc1667 head.
  • The first hosted run encountered an unrelated new-dev roster oracle that listed 13 models but still expected a count of 11. Upstream test: repair Windows-only and roster regressions surfaced by full-platform dev CI #5592 repaired that assertion while retaining the explicit model set. After merging that fix, bun run test -- ./tests/providers/provider-registry-parity.test.ts passed 59 tests, 0 failures, 2,297 assertions, 6.04 seconds on pinned Bun 1.4.0. No duplicate production or test fix was added to this feature.
  • All three source contributions were carried with -x provenance and original author/date. Two stable patch IDs match exactly; the third differs only in structure-table context and has an identical zero-context patch ID. The source-head dev merge adds no unique contribution beyond the carried source patch.
  • bun run test -- ./tests/oauth/oauth-public-surface.test.ts ./tests/providers/meta-muse-device.test.ts ./tests/providers/meta-muse-login-order.test.ts using pinned Bun 1.4.0: 76 passed, 0 failed, 278 assertions, 7.20 seconds. The mint-429 cancellation regression failed before the correction and passed afterward. Coverage includes principal/mode admission, no acquisition on rejection, bounded response failures and actual login dependency ordering using synthetic inputs.
  • Typecheck, structure SSOT, privacy scan, file-size ratchet and whitespace checks passed. The English/Chinese documentation build produced 497 pages, and the resulting admission/fallback text was read back. An initial absolute-config invocation failed; the corrected root-relative invocation succeeded without installing dependencies.
  • Exact-head hosted CI, full-suite/cross-platform completion and independent security review remain pending. No real provider, Keychain, account login or live runtime operation was performed. This PR stays draft.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

@coderabbitai

coderabbitai Bot commented Sep 22, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Sep 22, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 61 / 80

이 PR은 Meta Muse 로그인 관련 고친 내용을 하나로 모은 것이다. 예전 PR #5234와 #5432를 이어받아, 대시보드(관리 API)에서 Muse 로그인을 시작할 때는 서버가 확인한 gui-session이 있을 때만 로컬 키 가져오기나 기기 승인 흐름을 타게 한다. 관리용 토큰이나 가짜 GUI 헤더만으로는 403 oauth_consent_required로 막고, 그 전에 자격 증명을 읽거나 기기 로그인을 시작하지 않는다. 동시에 Muse의 기기 승인·토큰 폴링·키 발급 응답 본문을 64 KiB로 제한하고, 파싱하지 않는 오류 본문(예전에 빠졌던 mint 429 포함)은 읽어 들이지 않고 연결만 정리한다. 문서도 macOS에서 로컬 키를 먼저 쓰고, 없으면 기기/수동 입력으로 가는 순서를 맞춰 두었다. 기준 브랜치는 dev이고, 아직 draft다.

라인 - src/server/management/oauth-account-routes.ts의 403 문구가 “acknowledgement(확인/동의)”처럼 읽히는데, 같은 PR 문서·구조 설명은 “체크박스 영수증이 아니라 세션 principal”이라고 한다. 운영자가 대시보드에 체크박스가 있다고 오해할 수 있다.
라인 - PR 본문 체크리스트에 보안 리뷰 항목이 비어 있고, 작성자도 hosted CI·독립 보안 리뷰가 아직이라고 적어 두었다. draft인 점과 맞춰, merge 전에 그 게이트를 채울지 정해야 한다.
라인 - 원천 PR #5234·#5432는 이미 closed(미머지) 상태다. 이 통합 PR이 살아 있는 한 같은 수정을 다시 올리지 않도록 닫힌 상태를 유지하는 편이 맞다.

메인테이너의 판단이 필요한 지점

관리 API만 gui-session을 요구하고, 직접 ocx login meta-muse는 예전 정책을 그대로 둔다. Muse 자격 증명이 “지원하지 않는 사용”인데도 CLI 경로는 동의 게이트 밖이다. 제품으로 이 비대칭을 의도한 것인지, 아니면 CLI에도 같은 경고/게이트를 둘지 결정이 필요하다. 또한 오류 문구를 세션 요구로 바꿀지, 아니면 실제로 대시보드 확인 UI를 둘지 문서와 맞춰야 한다.

너의 추천

방향은 맞고 테스트도 admission → 자격 증명 획득 순서와 본문 크기 한도를 잘 묶었다. draft를 풀기 전에 (1) 403 문구를 “대시보드 세션 필요”처럼 문서와 같게 다듬고, (2) 보안 리뷰·CI를 채운 뒤, (3) #5234·#5432는 중복으로 다시 열지 말 것을 권한다. CLI 예외는 의도라면 문서에 한 줄로 더 분명히 남기고, 아니면 후속 이슈로 분리하면 된다.

이 댓글은 grok-bot이 작성했습니다

@lidge-jun

Copy link
Copy Markdown
Owner

Carried into #5619 at e1a9eb1 (same net patch as the earlier bffb533) with your authorship kept. A follow-up tightens the overflow test so it proves the body limit fired. Closing in favor of #5619. Thank you!

@lidge-jun lidge-jun closed this Sep 23, 2026
lidge-jun added a commit that referenced this pull request Sep 23, 2026
… fixes (#5619)

* fix(cursor): bound capability reads and buffered tool budgets (#5533)

Carries #5533 (and the closed #5233 it consolidates) onto current dev.

Co-authored-by: Epinephrine <luvs01@hanmail.net>

* fix(moonshot): bound normalized tool-schema expansion (#5547)

Carries #5547, which consolidates #5464 and the request-wide inline budget, onto current dev.

Co-authored-by: yeongjunyoo <47925973+yeongjunyoo@users.noreply.github.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Epinephrine <luvs01@hanmail.net>

* fix(moonshot): restore rejected inline budgets and charge nested growth once

A rejected sibling-reference expansion now restores the byte, node and expansion allowances it consumed, and outer growth no longer re-charges nested copies, so later independent expansions in the same request keep their allowance. Documents the provider-driven object type inference as a deliberate tradeoff and rewrites ADR-0355 in English.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* feat(reasoning): consolidate replay, opt-in tag parsing, and summary policy (#5566)

Carries #5566, which consolidates #5449, #5205 and #5491, onto current dev. The provider guide keeps the current bridge replay paragraph and adds the inline-tag and summary paragraphs.

Co-authored-by: Joonsuh Park <trckstr4422@gmail.com>
Co-authored-by: Daniel Sjöstrand <16033062+Danielsjostrand1979@users.noreply.github.com>
Co-authored-by: alexph-dev <alexph-dev@users.noreply.github.com>
Co-authored-by: Yum-wu <1172989563@qq.com>

* fix: bound Fernet slot runs, Kiro error-body read, and skill-path line slice (#5310)

Carries #5310 onto current dev. The follow-up commit makes the Fernet run cap fail closed and moves the Kiro regression out of the capped stream suite.

* docs(reasoning): reconcile inline-tag whitespace contract

Interleaved inline-tag parsing preserves answer whitespace; only Kiro single-block mode drops the whitespace after its leading block.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(responses): fail closed on Fernet run overflow and keep the Kiro suite under its cap

A slot with more than 64 structurally valid Fernet runs is now treated as unreadable or omitted as a whole, so no unexamined tail reaches the provider as text. The bounded Kiro fallback error-body regression moves byte for byte into a registered sibling file, and the Kiro, Responses and inbound contracts document the new bounds.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(reasoning): scan inline think tags with a moving cursor

The parser copied, rescanned and reserved the whole remaining response after every block, so one upstream chunk carrying many short blocks cost quadratic work. It now scans each chunk from an offset and charges the translator budget only for retained carry: undecided leading input or a trailing tag fragment.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(reasoning): keep undecided leading whitespace incremental

Before the format was decided, every content delta rebuilt, trimmed and re-reserved the whole leading prefix, so a stream of one-character whitespace deltas cost quadratic work. Leading whitespace is now kept in segments whose bytes are reserved once and joined only when the format is decided or the stream flushes.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(meta-muse): consolidate login admission and bounded response handling (#5591)

Carries #5591, which consolidates the closed #5234 and #5432, onto current dev. The provider contract keeps the inline-tag paragraph and adds the Meta Muse admission paragraph.

Co-authored-by: Epinephrine <luvs01@hanmail.net>

* fix(claude-desktop): keep applied state consistent across profile edits (#5590)

Carries #5590, which consolidates the closed #5337, onto current dev.

Co-authored-by: Epinephrine <luvs01@hanmail.net>
Co-authored-by: luvs01 <luvs01@users.noreply.github.com>

* fix(claude-desktop): commit applied markers only over the observed baseline

Both Desktop writers, provider-change auto-apply and client sync, now capture the desired profile and its applied marker before the Desktop write and commit the new marker only if profile presence, content, fingerprint and timestamp are unchanged. A concurrent edit, deletion or newer marker keeps its state and the write reports a skipped marker. The provider-change path no longer saves a whole stale config snapshot.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(claude-desktop): commit profile edits against the persisted marker

The Desktop profile PUT built its response from an earlier snapshot and saved that whole snapshot, so a marker committed by another writer during the awaited state build could be replaced by an older one. The edit now commits in one persisted-config mutation that keeps the latest marker for unchanged content and answers 409 when the profile itself changed meanwhile. The Meta Muse overflow test now asserts that the bounded-body limit, not a generic failure, produced the error.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(claude-desktop): report an unreadable config separately from an edit conflict

A missing or invalid config now answers 500 with its reason; only a concurrent profile change or exhausted rebase answers 409.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* feat(desktop): consolidate consent-based runtime takeover and ownership contracts (#5564)

Carries #5564, which consolidates #5459 and #5457, onto current dev. The review screenshot stays in the pull request description rather than the tree.

Co-authored-by: jun <bitkyc08@gmail.com>
Co-authored-by: sanggyulee <andy53295774@gmail.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

* fix(desktop): bind takeover stop to the approved runtime and fail closed

Desktop takeover re-resolves ownership immediately before stopping and passes the approved PID, endpoint, config home, CLI version and compatibility token to an opt-in guarded stop. The guard is checked under the ownership mutation lease before any manager or signal stop; the approved PID and endpoint must settle and the service manager must then be proven inactive, otherwise the stop answers approval-changed or manager-still-active and the desktop neither waits for silence nor claims. Unreadable or unparseable stop output is terminal as well. A second unreadable service-state read now blocks takeover, Windows managing-CLI discovery follows PATHEXT with file-only candidates and refuses command-interpreter metacharacters, the claim refusal test uses real sandbox state, and the runtime and desktop contracts record that the claim token is a consistency check rather than consent proof. Plain ocx stop is unchanged.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(desktop): keep plain stop entry points and format the takeover changes

Desktop exit keeps its plain runtime_stop::run entry while takeover uses run_approved, AttachPlan::Ask no longer carries an unread field, the Rust changes follow rustfmt, the plain CLI stop path keeps its literal outcome return, the stop source oracles follow the reader and outcome union that now include the two guarded refusals, and the runtime contract fits its 600-line budget.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(desktop): run takeover seam tests without tokio macros and harden manager and shim checks

The two async takeover seam tests now run on the shell runtime already used by the crate instead of tokio test macros, which this crate does not enable. Windows command-shim probes refuse command-interpreter metacharacters in every recorded argument as well as the executable, and the guarded stop re-inspects the service manager identity immediately before the manager command, answering approval-changed without stopping if it moved.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(responses): keep effort-based reasoning visible after routing

Final-route normalization recomputed hideThinkingSummary without the validated active-effort condition, so routed Chat and Kiro requests with an active effort and an omitted summary still hid raw reasoning. It now uses the same predicate as the parser; explicit "none" and requests without an active effort stay hidden.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(service): match the running CLI case-insensitively only on Windows

On case-sensitive filesystems a PATH executable that differs only in case is a different file, so it must get its own version probe instead of reporting the running CLI version.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(meta-muse): require the dashboard session for manual login codes

The manual-code continuation now applies the same dashboard-session admission as the login start, so a management token cannot advance a pending Meta Muse login.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(reasoning): reserve the joined leading-whitespace copy

Joining retained leading whitespace allocated a second copy outside the translator budget; the join is now reserved first and released once the segments are cleared.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(service): skip CLI probes for an absent runtime and treat failed systemd units as stopped

Resolve no longer spawns managing-CLI version probes when no runtime is live, since takeover is only offered for a live runtime. A systemd unit reported failed with no main PID is stopped, so a guarded stop that leaves it failed succeeds and a leftover failed unit does not block takeover.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* fix(service): keep failed systemd units fail-closed and assess takeover only for a live runtime in tests

systemd can report failed before an automatic restart, so failed with no main PID is again treated as unknown rather than stopped. The resolve contract tests that assert ownership and takeover fields now use a live runtime, matching the skip of managing-CLI probes when no runtime is live.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

---------

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
Co-authored-by: Epinephrine <luvs01@hanmail.net>
Co-authored-by: yeongjunyoo <47925973+yeongjunyoo@users.noreply.github.com>
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-authored-by: Joonsuh Park <trckstr4422@gmail.com>
Co-authored-by: Daniel Sjöstrand <16033062+Danielsjostrand1979@users.noreply.github.com>
Co-authored-by: alexph-dev <alexph-dev@users.noreply.github.com>
Co-authored-by: Yum-wu <1172989563@qq.com>
Co-authored-by: luvs01 <luvs01@users.noreply.github.com>
Co-authored-by: sanggyulee <andy53295774@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants