Skip to content

fix(audio): establish canonical local-audio resource policy - #866

Draft
seonghobae wants to merge 66 commits into
developfrom
fix/audio-resource-policy-781
Draft

fix(audio): establish canonical local-audio resource policy#866
seonghobae wants to merge 66 commits into
developfrom
fix/audio-resource-policy-781

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 16, 2026

Copy link
Copy Markdown
Collaborator

Buyer/security gap

Advances #781 by replacing divergent local-audio resource limits with one versioned policy and enforcing it before expensive analysis/model work. The same 100 MiB encoded-byte ceiling now applies to native local-file bootstrap, desktop bridge validation, Python request/decode admission, and YouTube download/bootstrap admission.

Exact current identity and stack

  • Protected integration base: develop@acdbea6344fe1231c39535b575f4de35e4c607c9.
  • Exact current head: d3e27929d794dd6333ca5458ecfa3ed705f3af52.
  • Branch: fix/audio-resource-policy-781.
  • Draft: retained.
  • Dependency predecessor: fix(score): bound native PDF reads before allocation #865 exact head f86e266b2ab2dc5a95e6b4a484e777b29f0feeaf at this refresh.

This branch deliberately descends from #865 because #865 owns the thin desktop-core crate root used by the native resource modules. #865 must integrate first; predecessor checks, reviews, and approval never transfer to this head.

Implemented policy-v1 boundary

  • Python AudioResourcePolicy: canonical 100 MiB encoded-file ceiling, 44.1 kHz target rate, 900-second decoded-duration ceiling, checked derived-sample arithmetic, mono/floating/finiteness requirements, and one-sample-over-budget decode probe.
  • Service request preflight validates localSource.fileSizeBytes before analysis dispatch.
  • TemporalAnalyzer and AudioStemSeparator decode only through the bounded policy probe and reject invalid output before beat analysis or Demucs/model work.
  • Desktop analysis.ts applies the same encoded-byte ceiling to native local-file and YouTube bootstrap responses and rejects non-integral JavaScript byte metadata.
  • Native desktop-core validates the filesystem-observed local/YouTube length before bootstrap state is stored.
  • YouTube download admission uses DEFAULT_MAX_ENCODED_FILE_BYTES: announced oversize rejects before download=True, yt-dlp max_filesize plus a progress hook abort in-flight, and the completed filesystem artifact is revalidated with the canonical policy.
  • In-flight oversize abort deletes only owned current-import .part, .ytdl, and -Frag* artifacts whose canonical path remains strictly inside that import's app-owned cache root; foreign, empty, directory, escaped, and path-resolution failures are ignored.
  • Current-head follow-up work extends duration/input revalidation and its regressions without changing dependency/root-lock ownership.
  • Errors remain payload-safe and omit source paths, URLs, cookies, and audio content.

Current-head convergence

The previous canonical head 5e8fa77f6ac1e38a68518285961da5056f22c242 is historical. The branch advanced by 17 commits to d3e27929d794dd6333ca5458ecfa3ed705f3af52, modifying only the audio-resource-policy/YouTube implementation, focused tests, CHANGELOG, and doctoring. The prior exact-head E501 failure in test_audio_resource_policy_coverage_regressions.py is therefore not current: the intervening branch work corrected that formatter/lint boundary and the unchanged current head now has terminal-success ci and release evidence.

All current inline review threads are resolved. The earlier Cursor CHANGES_REQUESTED findings about the 50 MiB post-write divergence and in-flight abort artifact leak are stale/currently addressed; later Cursor review explicitly verified those repairs on predecessor head 5e8fa77f. Two GitHub Code Quality test-helper findings are resolved/outdated on the current branch. Historical review state does not count as current independent approval.

Exact-current-head repository evidence

Evidence below binds only to d3e27929d794dd6333ca5458ecfa3ed705f3af52.

Terminal-success BandScope repository workflows:

  • ci run 31990978931;
  • release run 31990978948;
  • build-baseline run 31990978905;
  • sbom run 31990978911;
  • secret-scan-gate run 31990978903;
  • SAST Semgrep run 31990978893; and
  • bandit run 31990978900.

Two repository security workflows are non-passing, and their exact current-head jobs/logs were inspected before classification.

security-audit — inherited protected-base dependency baseline

Run 31990978877, job 95274388858, completes checkout/tool setup and npm ci, then fails specifically at npm audit --workspaces --audit-level=high on exactly the protected-base JavaScript findings already owned by canonical #783:

  • nanoid <3.3.18 (GHSA-2v37-7h3g-55p8);
  • pdfjs-dist >=5.6.83 <6.2.108 (GHSA-hq66-cqwq-w95j); and
  • undici 7.0.0–7.28.0 advisories.

The later Python/Rust audit stages are skipped because npm exits first; skipped evidence is not success. This branch has no package manifest/root-lock delta, so these findings are not #866-owned and are neither suppressed nor duplicated here.

aggregate Security Scan — inherited protected-base Trivy finding

Run 31990978916 has terminal-success Scorecard, Dependency Review, and OSV base-vs-head comparison. Its only failed substantive job is trivy-fs 95274390956, which reports exactly one HIGH whole-tree finding:

CVE-2026-16633 for pdfjs-dist at package-lock.json:6370.

The workflow itself directs remediation to the shared base so open PRs inherit the fix. This remains canonical #783-owned protected-base evidence, not a reason to mutate #866 product code or weaken a gate.

Ownership / dependency gates

Remaining #781 work

This Draft does not claim full #781 closure. Remaining lanes unless separately superseded by accepted architecture and current source evidence include:

  • complete source-channel/source-rate metadata contracts across all intake/analyzer boundaries;
  • decoded-memory estimates and CPU/GPU admission budgets; and
  • cancellation/resource measurements and whole-product CPU/GPU parity evidence.

Merge boundary

Keep Draft and unmerged until dependency-root order is satisfied and the unchanged resulting exact head has realistic focused/full tests GREEN, exact 100% owned production statement/branch coverage plus required docstrings/rustdoc, applicable native/Rust parity, every required SAST/security/SBOM/supply-chain/package/release/central review gate terminal-success, zero valid unresolved findings, #865/#783/#861/central prerequisites integrated and regenerated, a qualifying independent non-author last-push approval, and ordinary branch-protection acceptance without bypass.

Queued, skipped-required, failed, absent, predecessor-head, protected-base, model-only, self/author, or administrative-bypass evidence is not success.

Refs #781.

@coderabbitai

coderabbitai Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fdd698b4-5ad2-406f-8e5b-8c69ae1e6980

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

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>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale comment

Reviewed exact head 1f3fdb8b. The prior 50 MB / missing max_filesize / stale doctoring findings are fully addressed: YouTube download now uses DEFAULT_MAX_ENCODED_FILE_BYTES, rejects announced oversize before download=True, aborts from the progress hook, and revalidates the written file. Do not mark Ready or merge this draft until #865 is in protected develop and the abort-path cache leak below is on this head.

Request changes: in-flight abort still returns size_exceeded without deleting bytes already written. yt-dlp HttpFD writes the current block, then calls the hook; on exception it only closes the stream. The post-download path deletes an oversize final artifact; the abort path does not. Each rejected import can leave *.part, *-Frag*, and *.ytdl in a fresh project cache.

Successor cursor/bc-75568fe4-aa90-4cf7-bb40-c9d68be95b82-b46f at 5e8fa77f deletes owned siblings that stay inside that import out_dir and ignores escaped paths. Apply that here or reconstruct this branch onto it before Ready.

Queued, skipped, predecessor, or draft-skipped CodeRabbit evidence is not success.

Open in Web View Automation 

Sent by Cursor Automation: Fix Issues

Comment thread services/analysis-engine/src/bandscope_analysis/youtube.py

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed exact head 5e8fa77f on fix/audio-resource-policy-781 (base develop@acdbea63). The prior in-flight abort finding is fully addressed on this head: _abort_over_budget_download deletes owned siblings before the fail-closed size_exceeded raise. _owned_file_path realpaths the candidate and the import out_dir, rejects the directory root, and requires resolved.startswith(root + os.sep), so a path or symlink that escapes that import directory is ignored. _remove_download_artifacts stems tmpfilename / filename (one .part strip) and removes matching stem, stem.*, and stem-* entries, which covers .part, .ytdl, and -Frag*. test_download_youtube_audio_progress_hook_deletes_partial_artifacts proves those three are gone after abort while keep-me.txt and an outsider .part remain.

The earlier 50 MB post-write, missing Rust intake doctoring, CHANGELOG 50 MB, and progress-hook int-only items stay fixed. YouTube admission uses DEFAULT_MAX_ENCODED_FILE_BYTES (100 MiB) in Python, desktop analysis.ts, and native audio_resource.rs. Announced oversize rejects before download=True. Exact 100 MiB is accepted; 60 MiB is accepted; 100 MiB + 1 is rejected. Closed #875 is the same tree as this head — do not reopen a competing abort-cleanup owner.

Next action: keep this Draft. Integrate #865 into protected develop first, then reconstruct and revalidate this stack on the unchanged resulting exact head. Do not mark Ready or merge on queued, skipped, predecessor, or CodeRabbit draft-skipped evidence. Remaining #781 channel/rate contracts and decoded-memory / CPU/GPU admission budgets are still out of this draft's claim — do not treat policy-v1 encoded-byte admission as full #781 closure.

Residual (not a change request): a process kill, a locked Windows .part, or a differently named format-id fragment can still leave cache bytes until that per-project import directory is removed. Generic DownloadError / timeout paths do not sweep unnamed artifacts. Admission still fails closed.

Open in Web View Automation 

Sent by Cursor Automation: Fix Issues

Comment thread services/analysis-engine/tests/test_youtube_duration_contract.py Fixed
Comment thread services/analysis-engine/tests/test_youtube_duration_contract.py Fixed
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