Skip to content

fix(audio): abort YouTube downloads at the canonical 100 MiB ceiling - #871

Closed
cursor[bot] wants to merge 43 commits into
developfrom
cursor/bc-977eae6a-247d-427f-a2eb-533a75284f2e-6591
Closed

fix(audio): abort YouTube downloads at the canonical 100 MiB ceiling#871
cursor[bot] wants to merge 43 commits into
developfrom
cursor/bc-977eae6a-247d-427f-a2eb-533a75284f2e-6591

Conversation

@cursor

@cursor cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown

Summary

Continues #866 / #781. Native local-file and post-success YouTube bootstrap already used the 100 MiB policy, but download_youtube_audio still downloaded with no yt-dlp max_filesize and then rejected > 50MB. That rejected 60–100 MiB imports the new policy would accept and let a large transfer fill the cache root before the native check ran.

This branch keeps #866's policy stack and adds download-time admission:

  • max_filesize and a progress hook abort in-flight once encoded bytes exceed DEFAULT_MAX_ENCODED_FILE_BYTES.
  • Announced filesize / filesize_approx over the ceiling rejects before download=True.
  • Post-download AudioResourcePolicy.validate_encoded_file_bytes deletes leftovers.
  • User-facing oversize copy matches the local-file 100 MiB next-action message.
  • Doctoring residual-risk text no longer claims Rust intake is missing; yt-dlp evidence is cited in APA 7th.

A 60 MiB YouTube import is now accepted; a file one byte over 100 MiB is not.

Verification

  • Focused: PYTHONPATH=src python3 -m pytest tests/test_youtube.py tests/test_audio_resource_policy.py — 47 passed (26 YouTube + policy).
  • ruff check / ruff format --check on the YouTube module and tests.
  • mypy --strict --follow-imports=skip on youtube.py.
  • ./scripts/harness/quickcheck.sh (full harness not run in this environment; exact-head CI must pass before Ready).

Security Notes

Attack surface

YouTube URL intake, yt-dlp subprocess/download, cache-root writes, announced filesize metadata, progress-hook byte counters, post-download file deletion.

Trust boundary

Renderer JSON is not authoritative. URL allowlist stays first. Encoded-byte admission is the canonical Python policy. Native bootstrap still re-checks filesystem-observed length before storing project state.

Mitigations

  • max_filesize + progress-hook abort so unknown-size transfers cannot fill the cache root.
  • Pre-download reject on announced oversize; post-download policy revalidation and delete.
  • Payload-free errors (no paths, URLs, cookies, or audio content).
  • Boolean/non-finite announced sizes are ignored rather than trusted.

Test points

  • 60 MiB accepted (legacy 50 MB would reject).
  • Exact 100 MiB accepted; 100 MiB + 1 deleted.
  • Announced filesize / filesize_approx oversize never starts download.
  • Progress hook oversize and yt-dlp max-filesize errors map to size_exceeded without leaking raw messages.

Dependency and Supply Chain

  • No new direct dependency was added
  • If a new dependency was added, this PR explains why it is needed
  • runtime / dev / build / test classification is recorded
  • alternatives were considered
  • maintainer trust and update health were checked
  • license fit was checked
  • known security issues were checked
  • transitive footprint impact was considered
  • SBOM or supplemental inventory impact was recorded

i18n impact

  • No user-visible string changed
  • Korean and English locale impact was updated

The YouTube oversize engine message now matches the existing local-file 100 MiB copy already used by the desktop bridge. Locale files do not own this string.

Reviewer checklist

  • Gitflow target branch is develop
  • protected-branch rules were not weakened
  • required checks are expected to stay green

Stacked on #866 / #865. Do not merge until those ancestors are in protected develop or this head is reconstructed onto the resulting base. Refs #781.

Open in Web View Automation 

seonghobae and others added 13 commits August 16, 2026 22:29
A 60 MiB import must be accepted, exact 100 MiB must pass, and
announced/in-flight/post-download oversize must fail before cache fill.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
…ling

Drive yt-dlp max_filesize and a progress hook from AudioResourcePolicy,
reject announced oversize before download=True, and delete artifacts
that still exceed 100 MiB after write.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Update residual-risk text so Rust intake is no longer described as
missing, and cite yt-dlp max_filesize/progress_hooks in APA 7th.

Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>

Copy link
Copy Markdown
Collaborator

Superseded by the canonical existing #781 owner #866 after exact-head reconciliation. #866 now has merge head 1f3fdb8bfe869d747deaf4ce5cb53c23329e690f with #871 exact head 7b3b1e75ea8486869d0cc887122c9999ae2667fe as an explicit parent. compare(7b3b1e75…, 1f3fdb8…) is ahead by two commits and the only tree delta is #866's separate fail-closed coverage regression file services/analysis-engine/tests/test_audio_resource_policy_coverage_regressions.py; therefore every #871 source/test/doc/CHANGELOG change is preserved unchanged in #866. Closing this duplicate keeps one canonical writer for the audio-resource-policy boundary. Exact-head CI/review on #866 must still prove the integrated result before any Ready/merge classification.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants