From f52dc9738f48001acf88384a422db206b26bc727 Mon Sep 17 00:00:00 2001 From: seonghobae <8172694+seonghobae@users.noreply.github.com> Date: Sun, 6 Sep 2026 04:00:23 +0000 Subject: [PATCH 1/9] =?UTF-8?q?=E2=9A=A1=20Bolt:=20[performance=20improvem?= =?UTF-8?q?ent]=20Replace=20O(N^2)=20list=20membership=20checks=20with=20O?= =?UTF-8?q?(1)=20dictionary=20key=20deduplication=20in=20chart=20export?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .jules/bolt.md | 3 ++ .../src/bandscope_analysis/exports/chart.py | 31 +++++++++---------- .../tests/test_supply_chain_policy.py | 4 +-- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/.jules/bolt.md b/.jules/bolt.md index d54cf10fc..c0f9a50a7 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -61,3 +61,6 @@ ## 2026-07-13 - Array.from mapping optimization **Learning:** Using `Array.from({ length: N }).map(...)` creates an intermediate array of `undefined` values which requires memory allocation and garbage collection, adding O(N) unnecessary overhead in frequently re-rendered UI components. **Action:** Use `Array.from({ length: N }, (_, index) => ...)` to map elements directly during array creation, avoiding intermediate allocations. +## 2025-02-23 - Python O(N²) List Deduplication Anti-Pattern +**Learning:** Checking list membership (`if item not in lst: lst.append(item)`) inside loops creates a hidden O(N²) algorithmic bottleneck because `not in` on a list requires an O(N) scan. This can become a significant performance issue when analyzing songs with many sections or complex cue roles (e.g., in `chart.py` for chart export). +**Action:** When deduplicating strings or primitive items while preserving order in Python 3.7+, use dictionary key assignment (`dict_obj[item] = None`) inside the loop, and return `list(dict_obj.keys())` at the end. This reduces the complexity to O(N) by utilizing O(1) hashing for membership checks, without sacrificing readability. diff --git a/services/analysis-engine/src/bandscope_analysis/exports/chart.py b/services/analysis-engine/src/bandscope_analysis/exports/chart.py index 3a84b59c8..9a2a6da40 100644 --- a/services/analysis-engine/src/bandscope_analysis/exports/chart.py +++ b/services/analysis-engine/src/bandscope_analysis/exports/chart.py @@ -78,14 +78,14 @@ def _active_role_ids(section: Mapping[str, object]) -> list[str] | None: part_graph = section.get("partGraph") if not isinstance(part_graph, list): return None - active: list[str] = [] + active: dict[str, None] = {} for node in part_graph: if not isinstance(node, Mapping) or node.get("is_active") is not True: continue role_id = node.get("role_id") - if isinstance(role_id, str) and role_id and role_id not in active: - active.append(role_id) - return active + if isinstance(role_id, str) and role_id: + active[role_id] = None + return list(active.keys()) def _active_roles(section: Mapping[str, object]) -> list[Mapping[str, object]]: @@ -121,25 +121,25 @@ def _role_display_name(role: Mapping[str, object]) -> str | None: def _active_role_names(section: Mapping[str, object]) -> list[str]: """Return de-duplicated display names for the section's active roles.""" - names: list[str] = [] + names: dict[str, None] = {} for role in _active_roles(section): name = _role_display_name(role) - if name is not None and name not in names: - names.append(name) - return names + if name is not None: + names[name] = None + return list(names.keys()) def _section_cue(section: Mapping[str, object]) -> str: """Join the active roles' cue values into a single cue string.""" - cues: list[str] = [] + cues: dict[str, None] = {} for role in _active_roles(section): cue = role.get("cue") if not isinstance(cue, Mapping): continue value = cue.get("value") - if isinstance(value, str) and value and value not in cues: - cues.append(value) - return "; ".join(cues) + if isinstance(value, str) and value: + cues[value] = None + return "; ".join(cues.keys()) def _confidence_level(section: Mapping[str, object]) -> str | None: @@ -188,7 +188,7 @@ def _section_lines(sections: list[Mapping[str, object]]) -> list[str]: def _footer_lines(song: Mapping[str, object], sections: list[Mapping[str, object]]) -> list[str]: """Build the footer: per-role rehearsal priorities and the export focus.""" lines: list[str] = [] - priorities: list[str] = [] + priorities: dict[str, None] = {} for section in sections: for role in _section_roles(section): name = _role_display_name(role) @@ -196,11 +196,10 @@ def _footer_lines(song: Mapping[str, object], sections: list[Mapping[str, object if name is None or not isinstance(priority, str) or not priority: continue entry = f" - {name}: {priority}" - if entry not in priorities: - priorities.append(entry) + priorities[entry] = None if priorities: lines.append("Priorities:") - lines.extend(priorities) + lines.extend(priorities.keys()) summary = song.get("exportSummary") if isinstance(summary, Mapping): headline = summary.get("headline") diff --git a/services/analysis-engine/tests/test_supply_chain_policy.py b/services/analysis-engine/tests/test_supply_chain_policy.py index 1d8224c5a..6a0853944 100644 --- a/services/analysis-engine/tests/test_supply_chain_policy.py +++ b/services/analysis-engine/tests/test_supply_chain_policy.py @@ -1275,9 +1275,7 @@ def test_workflow_concurrency_cancels_only_superseded_pr_heads() -> None: workflow = (workflows_dir / workflow_name).read_text(encoding="utf-8") assert "concurrency:" in workflow, workflow_name assert "cancel-in-progress: false" in workflow, workflow_name - assert "contents: read" in workflow or "permissions: read-all" in workflow, ( - workflow_name - ) + assert "contents: read" in workflow or "permissions: read-all" in workflow, workflow_name assert "pull_request:" not in (workflows_dir / "release.yml").read_text(encoding="utf-8") From a9c2913f9ea52ec3f654b88a902b3ee10a9a4c6c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 13:08:21 +0900 Subject: [PATCH 2/9] test(security): preserve CSV C0 and full-width formula regressions --- apps/desktop/src/lib/export.test.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/apps/desktop/src/lib/export.test.ts b/apps/desktop/src/lib/export.test.ts index 265e983d4..e0e97ddd6 100644 --- a/apps/desktop/src/lib/export.test.ts +++ b/apps/desktop/src/lib/export.test.ts @@ -67,6 +67,26 @@ describe("export sanitization", () => { expect(escapeCsvField("\t+SUM(A1)")).toBe("'\t+SUM(A1)"); expect(escapeCsvField("\n-100")).toBe("\"'\n-100\""); expect(escapeCsvField("\r@cmd")).toBe("\"'\r@cmd\""); + + // Prevent bypasses using NUL bytes, including a NUL-only cell. + expect(escapeCsvField("\x00=1+2")).toBe("'\x00=1+2"); + expect(escapeCsvField(" \x00@cmd")).toBe("' \x00@cmd"); + expect(escapeCsvField("\x00\x00=1+2")).toBe("'\x00\x00=1+2"); + expect(escapeCsvField(" \x00\x00@cmd")).toBe("' \x00\x00@cmd"); + expect(escapeCsvField("\x00")).toBe("'\x00"); + + // Spreadsheet/parser disagreement is not limited to NUL: fail closed on any leading C0 control. + expect(escapeCsvField("\x1B+SUM(A1)")).toBe("'\x1B+SUM(A1)"); + expect(escapeCsvField(" \x07@cmd")).toBe("' \x07@cmd"); + expect(escapeCsvField("\x1B")).toBe("'\x1B"); + }); + + it("preserves the full-width operator regression contract from PR #941", () => { + expect(escapeCsvField("=1+2")).toBe("'=1+2"); + expect(escapeCsvField("+SUM(A1)")).toBe("'+SUM(A1)"); + expect(escapeCsvField("-100")).toBe("'-100"); + expect(escapeCsvField("@cmd")).toBe("'@cmd"); + expect(escapeCsvField(" \uFEFF=SUM(A1)")).toBe("' \uFEFF=SUM(A1)"); }); it("handles combined scenarios: formula injection with structural characters", () => { From 1157ba9739cef0b72810cc02a95e91b1902f376c Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 13:09:09 +0900 Subject: [PATCH 3/9] fix(security): neutralize CSV control-prefix formula cells --- apps/desktop/src/lib/export.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/desktop/src/lib/export.ts b/apps/desktop/src/lib/export.ts index 3d4493b1d..9d45ed6fc 100644 --- a/apps/desktop/src/lib/export.ts +++ b/apps/desktop/src/lib/export.ts @@ -11,7 +11,7 @@ import { // Security notes: // 1. Filename sanitization to prevent directory traversal or invalid characters. -// 2. CSV formula injection prevention (fields starting with =, +, -, @ must be prefixed with a single quote). +// 2. CSV formula injection prevention (dangerous ASCII/full-width formula or control initiators receive a single-quote prefix). /** Documented. */ export function sanitizeFilename(title: string): string { @@ -22,8 +22,9 @@ export function sanitizeFilename(title: string): string { /** Documented. */ export function escapeCsvField(value: string): string { let escapedValue = value; - // Prevent CSV formula injection by prefixing problematic leading characters with a single quote - if (/^[\s\uFEFF\xA0]*[=+\-@\t\r\n]/.test(value)) { + // Spreadsheet/parser disagreement can make a leading C0 control security-significant even when it precedes an operator. + // eslint-disable-next-line no-control-regex + if (/^[\s\uFEFF\xA0]*[\x00-\x1F=+\-@\uFF1D\uFF0B\uFF0D\uFF20]/.test(value)) { escapedValue = `'${value}`; } // Enclose in double quotes if there's a comma, newline, or double quote From f9875aed9403d6372297d107f78b2f7a0807ec10 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sun, 6 Sep 2026 13:09:36 +0900 Subject: [PATCH 4/9] docs(security): record CSV parser-disagreement boundary --- .jules/sentinel.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index 34122c2b4..f6a87d02f 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -6,7 +6,7 @@ ## 2024-07-07 - Unsanitized Directory Input Paths API Validation **Vulnerability:** The API logic allowed user-controlled local data directory paths (`cacheRoot` and `tempRoot`) to be directly used without mitigating cross-platform path traversal vulnerabilities. **Learning:** Checking for '..' sequences in untrusted paths fails to parse cross-platform separators reliably for untrusted inputs (e.g., Windows backslashes on POSIX). Relying solely on `os.sep` or `os.altsep` is inadequate because absolute paths can bypass restrictions if not resolved correctly, or if `os.altsep` is None. -**Prevention:** Manually replace backslashes with forward slashes and split by forward slash (e.g., `if '..' in path.replace('\\', '/').split('/')`) to enforce path traversal protections explicitly for restricted directory inputs provided via the API. Do not block `~` for user-selected input files. +**Prevention:** Manually replace backslashes with forward slashes and split by forward slash (e.g. `if '..' in path.replace('\\', '/').split('/')`) to enforce path traversal protections explicitly for restricted directory inputs provided via the API. Do not block `~` for user-selected input files. ## 2024-05-20 - Python Path Traversal Mitigation bypass **Vulnerability:** Path traversal detection in Python backend APIs relied solely on checking the input path string or basic parsed parts which might not adequately catch sequences like `..` when intermixed with different path separators. @@ -28,3 +28,8 @@ **Vulnerability:** The Rust backend (`apps/desktop/src-tauri/src/main.rs`) did not enforce a maximum URL length limit when processing YouTube URLs via `import_youtube_url`. While the frontend enforced `MAX_YOUTUBE_URL_LENGTH = 2000` via the input element, this could be bypassed by an attacker sending requests directly to the Tauri backend API, potentially causing a Denial of Service (DoS) due to unbounded URL parsing and regex matching. **Learning:** Input validation must occur at the entry point of untrusted data on the backend, even if it is also validated on the frontend. Relying solely on frontend validation for constraints like string length can expose the backend to resource exhaustion vulnerabilities. **Prevention:** Always enforce constraints like maximum length, format validation, and sanitization at the earliest possible point on the backend, typically at the API boundary, regardless of frontend safeguards. + +## 2026-09-05 - CSV Formula Injection C0 Control Prefix Bypass +**Vulnerability:** CSV formula-injection mitigation was incomplete when a cell began with a C0 control character (`\x00`-`\x1F`) that could be interpreted differently by downstream spreadsheet or parser implementations before a formula token. +**Learning:** NUL is only one member of the parser-disagreement boundary. Security policy must not depend on every downstream consumer preserving leading control bytes exactly, and executable regressions must include non-whitespace controls such as ESC as well as NUL. +**Prevention:** In `escapeCsvField`, treat any leading C0 control after permitted whitespace/BOM/NBSP as dangerous, prefix the entire original field before structural CSV quoting, and retain regressions for NUL-only, repeated NUL, whitespace+control, ESC-prefixed formula-shaped values, and full-width formula operators. Keep the lint exception scoped only to the intentional control-character regular expression. From 0725eb3ce0d5b416464566422839ff61c17b839e Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 07:05:17 +0900 Subject: [PATCH 5/9] repair(ci): drop superseded chart note from Ruff owner --- .jules/bolt.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/.jules/bolt.md b/.jules/bolt.md index c0f9a50a7..d54cf10fc 100644 --- a/.jules/bolt.md +++ b/.jules/bolt.md @@ -61,6 +61,3 @@ ## 2026-07-13 - Array.from mapping optimization **Learning:** Using `Array.from({ length: N }).map(...)` creates an intermediate array of `undefined` values which requires memory allocation and garbage collection, adding O(N) unnecessary overhead in frequently re-rendered UI components. **Action:** Use `Array.from({ length: N }, (_, index) => ...)` to map elements directly during array creation, avoiding intermediate allocations. -## 2025-02-23 - Python O(N²) List Deduplication Anti-Pattern -**Learning:** Checking list membership (`if item not in lst: lst.append(item)`) inside loops creates a hidden O(N²) algorithmic bottleneck because `not in` on a list requires an O(N) scan. This can become a significant performance issue when analyzing songs with many sections or complex cue roles (e.g., in `chart.py` for chart export). -**Action:** When deduplicating strings or primitive items while preserving order in Python 3.7+, use dictionary key assignment (`dict_obj[item] = None`) inside the loop, and return `list(dict_obj.keys())` at the end. This reduces the complexity to O(N) by utilizing O(1) hashing for membership checks, without sacrificing readability. From 1d38e8e62d66ebe6ad14df044cb29ede8e4ea7a6 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 07:05:46 +0900 Subject: [PATCH 6/9] repair(ci): return chart optimization to canonical owner --- .../src/bandscope_analysis/exports/chart.py | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/services/analysis-engine/src/bandscope_analysis/exports/chart.py b/services/analysis-engine/src/bandscope_analysis/exports/chart.py index 9a2a6da40..44e92005b 100644 --- a/services/analysis-engine/src/bandscope_analysis/exports/chart.py +++ b/services/analysis-engine/src/bandscope_analysis/exports/chart.py @@ -7,7 +7,7 @@ Security Notes: - Pure dict-to-string transformation: no file, network, or process I/O. - Never reads source-path fields and never emits filesystem paths. - - Safe failure: ``None``, empty, or malformed input yields ``""`` / ``[]``; + - Safe failure: ``None``, empty, or malformed input yields ``\"\"`` / ``[]``; missing or malformed keys are skipped and no exceptions escape. """ @@ -78,14 +78,14 @@ def _active_role_ids(section: Mapping[str, object]) -> list[str] | None: part_graph = section.get("partGraph") if not isinstance(part_graph, list): return None - active: dict[str, None] = {} + active: list[str] = [] for node in part_graph: if not isinstance(node, Mapping) or node.get("is_active") is not True: continue role_id = node.get("role_id") - if isinstance(role_id, str) and role_id: - active[role_id] = None - return list(active.keys()) + if isinstance(role_id, str) and role_id and role_id not in active: + active.append(role_id) + return active def _active_roles(section: Mapping[str, object]) -> list[Mapping[str, object]]: @@ -121,25 +121,25 @@ def _role_display_name(role: Mapping[str, object]) -> str | None: def _active_role_names(section: Mapping[str, object]) -> list[str]: """Return de-duplicated display names for the section's active roles.""" - names: dict[str, None] = {} + names: list[str] = [] for role in _active_roles(section): name = _role_display_name(role) - if name is not None: - names[name] = None - return list(names.keys()) + if name is not None and name not in names: + names.append(name) + return names def _section_cue(section: Mapping[str, object]) -> str: """Join the active roles' cue values into a single cue string.""" - cues: dict[str, None] = {} + cues: list[str] = [] for role in _active_roles(section): cue = role.get("cue") if not isinstance(cue, Mapping): continue value = cue.get("value") - if isinstance(value, str) and value: - cues[value] = None - return "; ".join(cues.keys()) + if isinstance(value, str) and value and value not in cues: + cues.append(value) + return "; ".join(cues) def _confidence_level(section: Mapping[str, object]) -> str | None: @@ -188,7 +188,7 @@ def _section_lines(sections: list[Mapping[str, object]]) -> list[str]: def _footer_lines(song: Mapping[str, object], sections: list[Mapping[str, object]]) -> list[str]: """Build the footer: per-role rehearsal priorities and the export focus.""" lines: list[str] = [] - priorities: dict[str, None] = {} + priorities: list[str] = [] for section in sections: for role in _section_roles(section): name = _role_display_name(role) @@ -196,10 +196,11 @@ def _footer_lines(song: Mapping[str, object], sections: list[Mapping[str, object if name is None or not isinstance(priority, str) or not priority: continue entry = f" - {name}: {priority}" - priorities[entry] = None + if entry not in priorities: + priorities.append(entry) if priorities: lines.append("Priorities:") - lines.extend(priorities.keys()) + lines.extend(priorities) summary = song.get("exportSummary") if isinstance(summary, Mapping): headline = summary.get("headline") @@ -215,7 +216,7 @@ def build_chart_text(song: Mapping[str, object] | None) -> str: section (``[mm:ss-mm:ss] LABEL (confidence) roles: ...``), and a footer with rehearsal priorities and the export focus headline. Output is deterministic and never contains filesystem paths. Malformed input - yields ``""``. + yields ``\"\"``. """ if not isinstance(song, Mapping): return "" From a7b0030a3a6cc6296a19ba3f8eaf595d470d05bd Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 07:06:39 +0900 Subject: [PATCH 7/9] repair(ci): restore protected chart bytes exactly --- .../analysis-engine/src/bandscope_analysis/exports/chart.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services/analysis-engine/src/bandscope_analysis/exports/chart.py b/services/analysis-engine/src/bandscope_analysis/exports/chart.py index 44e92005b..3a84b59c8 100644 --- a/services/analysis-engine/src/bandscope_analysis/exports/chart.py +++ b/services/analysis-engine/src/bandscope_analysis/exports/chart.py @@ -7,7 +7,7 @@ Security Notes: - Pure dict-to-string transformation: no file, network, or process I/O. - Never reads source-path fields and never emits filesystem paths. - - Safe failure: ``None``, empty, or malformed input yields ``\"\"`` / ``[]``; + - Safe failure: ``None``, empty, or malformed input yields ``""`` / ``[]``; missing or malformed keys are skipped and no exceptions escape. """ @@ -216,7 +216,7 @@ def build_chart_text(song: Mapping[str, object] | None) -> str: section (``[mm:ss-mm:ss] LABEL (confidence) roles: ...``), and a footer with rehearsal priorities and the export focus headline. Output is deterministic and never contains filesystem paths. Malformed input - yields ``\"\"``. + yields ``""``. """ if not isinstance(song, Mapping): return "" From e7d8a086e31ce9ad2b255f3200a313b45aaacdcc Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 07:36:31 +0900 Subject: [PATCH 8/9] chore(ci): verify restacked CSV boundary From 607957e24a3b1c32a5b4faddb551145c6460e301 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 7 Sep 2026 07:39:38 +0900 Subject: [PATCH 9/9] docs(security): record canonical prerequisite lineage --- .jules/sentinel.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.jules/sentinel.md b/.jules/sentinel.md index f6a87d02f..a92cae563 100644 --- a/.jules/sentinel.md +++ b/.jules/sentinel.md @@ -33,3 +33,5 @@ **Vulnerability:** CSV formula-injection mitigation was incomplete when a cell began with a C0 control character (`\x00`-`\x1F`) that could be interpreted differently by downstream spreadsheet or parser implementations before a formula token. **Learning:** NUL is only one member of the parser-disagreement boundary. Security policy must not depend on every downstream consumer preserving leading control bytes exactly, and executable regressions must include non-whitespace controls such as ESC as well as NUL. **Prevention:** In `escapeCsvField`, treat any leading C0 control after permitted whitespace/BOM/NBSP as dangerous, prefix the entire original field before structural CSV quoting, and retain regressions for NUL-only, repeated NUL, whitespace+control, ESC-prefixed formula-shaped values, and full-width formula operators. Keep the lint exception scoped only to the intentional control-character regular expression. + +**Repair lineage:** The inherited `test_supply_chain_policy.py` formatting prerequisite remains owned by PR #1176 at `a7b0030a3a6cc6296a19ba3f8eaf595d470d05bd`; this security branch integrates that exact commit as ancestry instead of copying a competing edit.