Fix stored XSS in retune modal

[carochacs]

What

Security audit fix: retuneSong(filename, title, tuning, target) in static/app.js injected title/target/msg.filename/msg.error into a modal's innerHTML via unescaped template literals.

song.title is attacker-influenceable (imported GP/MusicXML/sloppak metadata) and reaches this sink directly from the library card's "Convert to E Standard" menu action, executing before any network round-trip. Since the app has no auth model (single-user, all endpoints trusted-local by design), script execution here grants full read/write access to the library via same-origin API calls.

Fixed by wrapping all four values in the codebase's existing esc() helper, matching the escaping pattern already used everywhere else in the file.

feedpak surface

• This PR does not change how the app reads/writes feedpaks (manifest keys, pack files, folder layout)

Checklist

• CHANGELOG.md [Unreleased] updated (user-visible changes)
• No new behavior — pure security fix, existing tests cover the affected code path
• Commits are DCO signed off (git commit -s)

---

Generated by Claude Code

Greptile Summary

The PR closes the stored-XSS path in the retune modal and documents the security fix.

• Escapes the song title and target before rendering the initial modal.
• Escapes server-provided filenames and errors before rendering completion states.
• Adds an Unreleased Security changelog entry describing the affected flow and remediation.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
static/app.js	Applies the existing HTML text-context escaping helper to all four dynamic values inserted into the retune modal.
CHANGELOG.md	Adds an accurate Security entry describing the stored-XSS vulnerability and fix.

_{Reviews (2): Last reviewed commit: "Update changelog for the retune-modal XS..." | Re-trigger Greptile}

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

⚠️ Agent ran but produced no code changes.

The CI failures may require manual intervention.

View agent analysis

Now let's check the commit subjects for the core-commit-labeling rule.

I investigated the failing check and the root cause is **not** a code/test bug — it's a commit-message policy gate.

## Root cause

This repo enforces a fork-maintenance policy (`docs/fork-maintenance.md`, Rule 2) via `.github/workflows/fork-audit.yml`'s `core-commit-labeling` job: **any commit introduced by a PR that touches a "core" path** (everything except `plugins/**`, `docs/**`, `tests/**`, `scripts/**`, `specs/**`, `.specify/**`, `CHANGELOG.md`, `VERSION`) **must have a subject line prefixed `core:`, `hook:`, or `sync:`**.

The PR has two commits ahead of `origin/main`:

| Commit | Files | Subject | Verdict |
|---|---|---|---|
| `2afd464` | `static/app.js` | `fix(security): escape song title/filename/error in retune modal HTML` | Touches a core path (`static/app.js`), subject doesn't start with `core:`/`hook:`/`sync:` → **fails the gate** |
| `33962cd` | `CHANGELOG.md` | `Update changelog for the retune-modal XSS fix` | Only touches an excluded path → exempt |

I verified `static/app.js`'s content is already correct and complete (all four values — `target`, `title`, `msg.filename`, `msg.error` — are properly routed through `esc()`, matching the PR's stated intent). There is no code defect to fix here.

Per `docs/fork-maintenance.md`'s own decision tree, a direct edit to a core file like this is the explicit **P3 exception** ("a real bug fix... label the commit `core:`"), which is exactly this case (a security bug fix, not a code smell to revert).

## Why I'm not making a file edit

The fix this check requires is **rewording the commit subject** of `2afd464` (e.g. to `core: fix(security): escape song title/filename/error in retune modal HTML`), not a change to any file's contents. That requires a commit-history operation (`git commit --amend` / non-interactive rebase to reword), which is explicitly out of scope for this task ("Do NOT run git commit or any command that creates a commit — only make file changes"). There's no file-content change that can satisfy a `git log`-based commit-message gate.

**Recommended remediation (to be applied outside this constrained session):**
```
git rebase -i origin/main   # mark 2afd464 as "reword"
# change subject to:
# core: fix(security): escape song title/filename/error in retune modal HTML
```
No other files need modification — `static/app.js` and `CHANGELOG.md` are already correct for the PR's stated purpose.

[coderabbitai]

Oops, something went wrong! Please try again later. 🐰 💔

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. 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: 93c5230e-83dc-4cae-b117-d451bb3af9d2

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

---

_{Comment @coderabbitai help to get the list of available commands.}

[pullfrog]

Fixed the failing core-commit-labeling check on PR #44.

Diagnosis — the check-suite failure (job core-commit-labeling, .github/workflows/fork-audit.yml) was a fork-commit-labeling violation, not a code failure. The workflow's regex only accepts subjects prefixed ^(core|hook|sync)(\([^)]*\))?:, and two commits this PR introduces touch core paths (everything outside plugins/ docs/ tests/ scripts/ specs/ .specify/ CHANGELOG.md VERSION) without a valid prefix:

• 2afd464 — fix(security): escape song title/filename/error in retune modal HTML touches static/app.js; fix(security): is not in the allowed prefix set.
• 071a797 — Fix remaining security-audit findings: XML DoS, zip bomb, missing headers touches lib/**, server.py, requirements.txt.

(33962cd only touches CHANGELOG.md, which is excluded — not flagged.) The failure is directly attributable to this PR's own commits; no code bug.

Fix — rewrote the two offending commit subjects to carry valid prefixes and force-pushed the feature branch (history rewrite is the only way to satisfy this check; adding a commit can't retrofit earlier ones):

• 2afd464 → core(security): escape song title/filename/error in retune modal HTML
• 071a797 → core: Fix remaining security-audit findings: XML DoS, zip bomb, missing headers

Verification — re-ran the exact CI step (rev-list e995a66..HEAD + prefix grep) locally: passes (exit=0). Confirmed content-identical to the pre-rewrite branch (tree hash bf38c194… unchanged) and clean working tree. Pushed to origin/claude/security-issues-jdpj26 (now ec713b4); the existing PR #44 picks up the rewrite on its next check run.

Task list (7/7 completed)

• Checkout PR Fix stored XSS in retune modal #44 branch
• Fetch check suite logs to diagnose failure
• Verify failure was introduced by this PR
• Diagnose and fix the issue
• Verify fix by re-running exact CI command
• Commit and push to existing PR
• Report progress with diagnosis and fix summary

  ^{｜ View workflow run ｜ via Pullfrog ｜ Using Big Pickle (free) ｜ 𝕏}