Skip to content

fix(oauth): bound Meta Muse device response bodies - #5432

Draft
luvs01 wants to merge 2 commits into
lidge-jun:devfrom
luvs01:fix/540-meta-muse-bound
Draft

luvs01 wants to merge 2 commits into
lidge-jun:devfrom
luvs01:fix/540-meta-muse-bound

Conversation

@luvs01

@luvs01 luvs01 commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

Motivation

  • Meta Muse device authorization, polling, and key-mint JSON responses were read without a shared size bound, so an oversized declared or streamed body could be buffered and parsed.

Description

  • src/oauth/meta-muse-device.ts applies the shared 64 KiB bounded-body ceiling and the request deadline to the device-authorization, polling, and key-mint JSON reads; oversized declared or streamed bodies are rejected before JSON parsing.
  • structure/providers-and-adapters.md documents the ceiling on the src/oauth/ row (the provider table moved there since the original patch).
  • Regression coverage in tests/providers/meta-muse-device.test.ts asserts oversized bodies are refused.

Testing

  • bun test tests/providers/meta-muse-device.test.ts: 39 tests pass.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Sep 21, 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 github-actions Bot added the intake: hygiene-blocked Deterministic PR hygiene checks failed label Sep 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Deterministic hygiene checks failed.

  • unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/oauth/meta-muse-device.ts.

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

github-actions Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • hygiene: unsponsored_surface.

What to do

  • Fix unsponsored_surface — This changes an authentication, workflow, release-automation, or dependency surface. MAINTAINERS.md requires security review for these; ask a maintainer to apply maintainer-sponsored once they have reviewed it. Paths: src/oauth/meta-muse-device.ts.
  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ All CI tests are green on my local testing.
  • ⬜ I pushed my PR to the latest dev commit.
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.
@luvs01 Tick the boxes once your local CI is green, your branch is on the latest dev commit, and every correct Codex and CodeRabbit finding is resolved.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 36 / 80

이 PR은 Meta Muse 로그인 응답이 너무 커도 그대로 JSON으로 읽던 구멍을 막습니다. 기기 코드를 받을 때, 승인을 기다릴 때, 구독 키를 받을 때, 본문이 65536바이트를 넘으면 파싱하지 않고 거절합니다. 길이 헤더가 이미 그보다 크면 받기 전에 끊습니다. 헤더가 없거나 더 작으면 바이트를 세면서 읽고, 그 읽기에도 요청 제한 시간 20초가 같이 걸립니다. 문서에 이 상한을 한 줄 적었습니다. 테스트도 세 개 넣었습니다. base는 dev입니다.

라인 tests/providers/meta-muse-device.test.ts 폴링 테스트 rejects an oversized token response instead of polling again - 실패 종류가 device-token이고 토큰 주소를 한 번만 쳤는지만 확인합니다. 크기 검사를 빼도, 200인데 access_token이 없으면 같은 종류로 바로 끝납니다. “65536-byte limit” 문장은 안 봅니다. 기기 코드 테스트와 키 발급 테스트는 그 문장을 보므로 그쪽은 상한이 없으면 떨어집니다.

라인 src/oauth/meta-muse-device.ts requestMuseDeviceAuthorization, mintMuseApiKey - 상태 코드가 실패면 readMuseJson을 호출하지 않습니다. 키 발급 실패 본문에는 키가 들어 있을 수 있어서, 읽지 않는 선택은 맞습니다. 다만 그 본문을 cancel하지도 않습니다. 성공 응답만 상한이 있습니다.

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

이 변경은 로그인 코드라서, 자동 검사가 maintainer-sponsored 라벨을 요구합니다. 라벨이 없어서 PR은 Draft로 남아 있고, 타깃 검사도 그 이유로 실패합니다. base dev 자체는 맞습니다. 실패 응답은 지금처럼 읽지 않을지, 읽지 않고 바로 cancel만 할지만 정해 주세요.

너의 추천

본문을 64KiB에서 자르는 방향은 맞으니 유지하세요. 폴링 테스트에 상한 문장 검사를 추가하세요. 실패 응답은 파싱하지 말고 cancel만 하는 쪽이 더 안전합니다. 메인테이너 라벨을 붙인 다음 Ready로 올리면 됩니다. types.ts/config.ts 분할과는 무관하고, 같은 수정의 다른 열린 PR은 없습니다.

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

@luvs01

luvs01 commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator Author

Review feedback applied on ecfd3f0805:nn- The polling test now asserts the 65536-byte limit message, so a 200-without-token path can no longer satisfy it.n- Both unparsed failure paths (device authorization, key mint) now cancel() the unread body so the connection is released; the bodies stay unparsed as before.nnTests: meta-muse-device39 pass. Auth surface -maintainer-sponsored` label still required before ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working intake: hygiene-blocked Deterministic PR hygiene checks failed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants