fix(api): bound inbound control request bodies - #799
Open
Quick104 wants to merge 1 commit into
Open
Conversation
|
Warning Review limit reachedNext included review available in 37 minutes. View limit detailsLimit details: You’ve used the included review currently available. Your 94 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Related issue: N/A — audit-validated security fix
Plugin HTTP routes, webhook-sync deliveries, and Jellyfin control requests could buffer or parse request bodies without a byte ceiling. A public plugin route made the strongest case unauthenticated; authenticated plugin routes, signed webhooks, PlaybackInfo, and Capabilities shared the same missing boundary. Chunked bodies bypassed
Content-Length-based screening, and Plex multipart parsing could spill an unbounded aggregate body to temporary disk.Fresh verification against
origin/mainat8164fd594b9fdd8c1944bb0b6251f2d00e4a24careproduced a 3 MiB + 1 byte chunked public-plugin request reaching the plugin and returning 200 before this patch.Approach
Add
internal/requestbodyas the shared byte-counted read boundary. It useshttp.MaxBytesReader, reads through the enforced limit, closes the transport body, and distinguishes overflow from other read failures. Bodies are rejected rather than truncated.The limits reflect each control surface:
Plugin descriptor and access checks still run before the body read. A valid webhook secret is resolved before reading, preserving unknown-secret 404 behavior, but overflow is rejected before provider parsing, event logging, or watch-state mutation. All four PlaybackInfo routes and both Capabilities aliases share the Jellyfin limit. Jellyfin debug logging now captures a prefix while forwarding the complete body instead of truncating what the handler receives.
No native client contract changed, and no Apple or Android follow-up is needed. Jellyfin compatibility is handled directly. Upload and media-stream endpoints were reviewed and left unchanged because their large-body semantics are intentional or already have dedicated budgets. There is no published API document for these oversized control-body errors to update.
Validation
Focused and adversarial checks:
The complete Linux Go suite passed against the exact candidate mirror on dev-builder:
The local macOS
make test-gorun reached two pre-existing process-lock failures ininternal/jellycompat:TestBeginWebOperationRecoversDeadProcessLockandTestBeginWebOperationRejectsLiveProcessLock. The same failures reproduce on untouchedorigin/main; the exact Linux candidate run passed them.The remaining repository gate passed:
Live validation used the isolated
audit-body-budgetdev-builder sandbox with the exact candidate.doctorpassed before testing. A registered synthetic plugin and API-created webhook connections produced:The sandbox was torn down after validation. Tokens, webhook secrets, private hostnames, and media details were not printed.
Risks
Clients or plugins sending unusually large control documents will now receive 413. The limits have substantial headroom over observed legitimate payloads, but a legitimate integration above a cap would need a narrowly justified limit adjustment. Rollback is the single commit in this PR; there is no migration or persisted-format change.
AI Disclosure
gpt-5.6-solChecklist