Skip to content

fix(audio): delete owned YouTube partials on in-flight abort - #875

Closed
cursor[bot] wants to merge 46 commits into
developfrom
cursor/bc-75568fe4-aa90-4cf7-bb40-c9d68be95b82-b46f
Closed

fix(audio): delete owned YouTube partials on in-flight abort#875
cursor[bot] wants to merge 46 commits into
developfrom
cursor/bc-75568fe4-aa90-4cf7-bb40-c9d68be95b82-b46f

Conversation

@cursor

@cursor cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown

Summary

Successor to #866 at exact head 1f3fdb8b. That head already uses the canonical 100 MiB encoded-byte ceiling (max_filesize, announced-size reject, progress-hook abort, post-download policy check). This change deletes owned .part, .ytdl, and -Frag* siblings that stay inside that import's output directory when the hook aborts, so a rejected transfer cannot accumulate cache bytes. Paths that escape the directory are ignored.

Apply this onto #866 or reconstruct #866 onto this head before Ready. #865 must still integrate first. Do not merge this draft as a substitute for #866's remaining #781 lanes.

Exact head after this commit: 5e8fa77f.

Verification

  • uv run pytest tests/test_youtube.py --cov=bandscope_analysis.youtube --cov-branch — 30 passed, 100% statement+branch on youtube.py
  • ruff check / ruff format / mypy --strict on the YouTube module
  • ./scripts/harness/quickcheck.sh — not a substitute for exact-head CI on this branch

Security Notes

Attack surface

YouTube import writes untrusted bytes into a project cache directory. An over-budget transfer can still write the current block before the progress hook raises.

Trust boundary

Filesystem cleanup is limited to paths that realpath to a file strictly inside the import out_dir. Renderer metadata and announced yt-dlp sizes remain untrusted.

Mitigations

  • Abort still uses DEFAULT_MAX_ENCODED_FILE_BYTES
  • Owned tmpfilename / filename and matching siblings are deleted on abort
  • Foreign, empty, directory, and unresolvable paths are ignored
  • Public errors stay payload-free

Test points

  • Abort deletes .part, -Frag1, and .ytdl siblings and keeps unrelated files inside and outside out_dir
  • Empty, escaped, and OSError path resolution fail closed
  • Remove/listdir races do not raise or delete foreign files

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

Reviewer checklist

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

Refs #781. Depends on #865. Repairs remaining abort-path leak on #866.

Open in Web View Automation 

seonghobae and others added 16 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>
Aborting at the 100 MiB ceiling still left .part, .ytdl, and -Frag*
siblings in the import cache. Delete only paths that stay inside that
import directory so a rejected transfer cannot accumulate cache bytes.

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

Copy link
Copy Markdown
Collaborator

Canonicalization evidence: #875 exact head 5e8fa77f6ac1e38a68518285961da5056f22c242 was exactly one commit ahead of canonical audio-resource owner #866 head 1f3fdb8bfe869d747deaf4ce5cb53c23329e690f (ahead_by=1, behind_by=0). The unique delta is bounded to youtube.py, test_youtube.py, docs/doctoring/audio-resource-policy.md, docs/security/app-security.md, and CHANGELOG.md. Fresh call-site inspection confirms each YouTube import receives a newly minted project ID and app-owned cache_root, so the cleanup remains inside that import's project cache. The canonical #866 branch was then fast-forwarded non-destructively to this exact commit. A fresh compare now reports status=identical, ahead_by=0, behind_by=0, and zero file differences between #875 and #866. Closing #875 as a proven duplicate/successor; all useful source, tests, security notes, and changelog evidence are preserved byte-for-byte in #866. No protected branch was changed.

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