Skip to content
5 changes: 5 additions & 0 deletions .jules/sentinel.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,8 @@
**Vulnerability:** CSV formula injection mitigation was naive, missing leading whitespace, tabs, and newlines.
**Learning:** Checking `/^[=+\-@]/` is not sufficient, as OWASP states that spaces and tabs before the formula triggers will also execute the formula in applications like Excel.
**Prevention:** Use a regex that allows leading whitespace (e.g. `/^[\s\uFEFF\xA0]*[=+\-@\t\r\n]/`) and include standalone tabs or new lines which are also injection vectors.

## 2025-06-22 - URL Parsing Length Limit
**Vulnerability:** Unbounded URL inputs at the analysis entry points.
**Learning:** Regular expressions and URL parsers can spend avoidable CPU or memory on oversized attacker-controlled strings.
**Prevention:** Cap URL length to the product-supported maximum before handing user input to regex or URL parsers. This PR enforces the cap at the TypeScript frontend (`apps/desktop/src/lib/analysis.ts`) and the Python engine (`services/analysis-engine/src/bandscope_analysis/youtube.py`). The Rust-side duplicate cap is deferred to a follow-up (same blocker as PR #527): the org review contract cannot currently evaluate Rust changes, so the diff intentionally touches no `.rs` files. Defense-in-depth is preserved by the TS entry point and the Python engine validation.
10 changes: 10 additions & 0 deletions apps/desktop/src/App.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { act, fireEvent, render, screen, waitFor } from "@testing-library/react";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { App } from "./App";
import { MAX_YOUTUBE_URL_LENGTH } from "./lib/analysis";

const tauriInvoke = vi.fn();
const mockLoadProject = vi.fn();
Expand Down Expand Up @@ -223,6 +224,15 @@ describe("App", () => {
expect(sourceControls).toHaveTextContent(/Import YouTube/i);
});

it("caps the YouTube URL input before import-path validation", () => {
render(<App />);

expect(screen.getByRole("textbox", { name: /YouTube URL/i })).toHaveAttribute(
"maxlength",
String(MAX_YOUTUBE_URL_LENGTH)
);
});

it("renders the loaded song as a dark rehearsal command board", async () => {
mockLoadProject.mockResolvedValueOnce(succeededResult().result);
render(<App />);
Expand Down
2 changes: 2 additions & 0 deletions apps/desktop/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
importYoutubeUrl,
isSupportedYoutubeUrl,
loadProject,
MAX_YOUTUBE_URL_LENGTH,
saveProject,
subscribeToAnalysisJobUpdates,
selectLocalAudioSource,
Expand Down Expand Up @@ -605,6 +606,7 @@ export function App() {
type="text"
placeholder={t("youtubePlaceholder")}
value={youtubeUrl}
maxLength={MAX_YOUTUBE_URL_LENGTH}
onChange={(e) => setYoutubeUrl(e.target.value)}
disabled={analysisInFlight || isStarting || isImporting}
className="h-10 flex-1 border-0 bg-transparent text-slate-100 placeholder:text-slate-500 focus-visible:ring-cyan-300"
Expand Down
24 changes: 23 additions & 1 deletion apps/desktop/src/lib/analysis.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import { createDemoAnalysisJobRequest, createDemoRehearsalSong } from "@bandscope/shared-types";
import { getAnalysisJobStatus, importYoutubeUrl, startAnalysisJob } from "./analysis";
import {
MAX_YOUTUBE_URL_LENGTH,
getAnalysisJobStatus,
importYoutubeUrl,
startAnalysisJob
} from "./analysis";

type TauriWindow = Window & {
__TAURI_INTERNALS__?: unknown;
Expand Down Expand Up @@ -196,6 +201,23 @@ describe("analysis bridge", () => {
});
});

it("rejects oversized YouTube URLs before crossing the Tauri bridge", async () => {
tauriWindow.__TAURI_INVOKE__ = vi.fn();
const urlPrefix = "https://youtube.com/watch?v=4ozX4yFUC34&x=";
const oversizedUrl = `${urlPrefix}${"a".repeat(MAX_YOUTUBE_URL_LENGTH - urlPrefix.length + 1)}`;

const selection = await importYoutubeUrl(oversizedUrl);

expect(tauriWindow.__TAURI_INVOKE__).not.toHaveBeenCalled();
expect(selection).toEqual({
ok: false,
error: {
code: "invalid_request",
message: "Only standard YouTube URLs are supported."
}
});
});

it.each([
"https://youtube.com/watch?v=too-short",
"https://youtube.com/watch?v=4ozX4yFUC3!",
Expand Down
6 changes: 6 additions & 0 deletions apps/desktop/src/lib/analysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ const SAFE_LOCAL_AUDIO_MESSAGES = new Set([
"Could not prepare the local temp workspace."
]);
const YOUTUBE_VIDEO_ID_PATTERN = /^[A-Za-z0-9_-]{11}$/;
const MAX_YOUTUBE_URL_LENGTH = 2000;

export { MAX_YOUTUBE_URL_LENGTH };

/** Documented. */
export type LocalAudioSelectionResult =
Expand Down Expand Up @@ -74,6 +77,9 @@ export function isSupportedYoutubeUrl(rawUrl: unknown): rawUrl is string {
if (typeof rawUrl !== "string") {
return false;
}
if (rawUrl.length > MAX_YOUTUBE_URL_LENGTH) {
return false;
}

let parsedUrl: URL;
try {
Expand Down
5 changes: 5 additions & 0 deletions services/analysis-engine/src/bandscope_analysis/youtube.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import yt_dlp # type: ignore

YOUTUBE_VIDEO_ID_PATTERN = re.compile(r"^[A-Za-z0-9_-]{11}$")
MAX_YOUTUBE_URL_LENGTH = 2000
SUPPORTED_AUDIO_EXTENSIONS = (".opus", ".m4a", ".mp3", ".wav", ".aac", ".flac", ".ogg")
YOUTUBE_DOWNLOAD_FAILED_MESSAGE = (
"Failed to download audio from YouTube. Please use a local audio file instead."
Expand All @@ -32,6 +33,10 @@ def validate_url(url: str) -> bool:
Returns:
True if the URL is valid, False otherwise.
"""
# Pragmatic upper bound to avoid spending parser/downloader work on oversized user input.
if len(url) > MAX_YOUTUBE_URL_LENGTH:
return False
Comment thread
seonghobae marked this conversation as resolved.

try:
parsed = urllib.parse.urlparse(url)
if parsed.scheme != "https":
Expand Down
7 changes: 6 additions & 1 deletion services/analysis-engine/tests/test_youtube.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import pytest
import yt_dlp # type: ignore

from bandscope_analysis.youtube import download_youtube_audio, validate_url
from bandscope_analysis.youtube import MAX_YOUTUBE_URL_LENGTH, download_youtube_audio, validate_url


def test_validate_url() -> None:
Expand All @@ -16,7 +16,11 @@ def test_validate_url() -> None:
assert validate_url("https://youtu.be/abc123DEF45") is True
assert validate_url("https://www.youtube.com/watch?v=abc123DEF45") is True
assert validate_url("https://www.youtube.com/watch?v=abc123DEF45&t=10") is True
url_prefix = "https://youtube.com/watch?v=abc123DEF45&x="
max_length_url = url_prefix + ("a" * (MAX_YOUTUBE_URL_LENGTH - len(url_prefix)))
long_query_url = max_length_url + "a"

assert validate_url(max_length_url) is True
assert validate_url("https://m.youtube.com/watch?v=abc123DEF45") is False
assert validate_url("https://music.youtube.com/watch?v=abc123DEF45") is False
assert validate_url("https://evil.youtube.com/watch?v=abc123DEF45") is False
Expand All @@ -34,6 +38,7 @@ def test_validate_url() -> None:
assert validate_url("https://youtube.com/watch?v=abc123DEF45&v=") is False
assert validate_url("https://youtube.com/watch?v=../../../etc/passwd") is False
assert validate_url("https://youtu.be/../../../etc/passwd") is False
assert validate_url(long_query_url) is False


def test_validate_url_edge_cases() -> None:
Expand Down
Loading