Static audit: ponytail pass + security review

[fastermadman]

Model: Opus 5 · Effort: high
Judgment-heavy, not mechanical: "is this complexity earned" and "is this actually exploitable" both require reasoning about intent and attack surface, not pattern-matching against a checklist.

Role

You are an adversarial reviewer, not the implementer. Every line of this codebase was written by an AI across several sessions with no independent review — treat the existing green test suite as unproven, not as evidence of correctness. Your job is to find what's wrong, not to confirm what's right.

Context (read first — assume no prior knowledge of this project)

beetsGUI is a local web app for beets, the command-line music library manager. Runs as a Safari Web App on macOS. No build step, no npm, no framework:

• server.py (~506 lines) — Flask on http://localhost:1312, binds to 127.0.0.1 only.
• importsession.py (~495 lines) — drives beets' importer in-process via a subclassed ImportSession.
• beetsgui.html (~1253 lines) — everything inline: HTML, CSS, JS. Vanilla JS, no framework.
• test_importsession.py (~432 lines) — end-to-end tests against a throwaway BEETSDIR, using ffmpeg-generated silent audio.

Routes currently defined in server.py: /config (GET/POST), /library, /unimported, /playlists, /, /assets/<path>, /status, /import/start, /import/current, /import/<id>/events, /import/<id>/decide, /import/<id>/abort, /run.

The project went through a deliberate rework (see git log, commits from 9fe8995 through 93124ca) turning a shell-command composer into an app that executes in-process. This audit checks whether that rework actually delivered what it claims.

Scope

1. Ponytail audit — whole repo, not a diff.

Run the equivalent of /ponytail-audit (or the same reasoning by hand) over all four files. Specifically look for:

• Dead code left behind by repeated refactors across sessions (functions, CSS classes, or HTML ids that nothing references anymore — a targeted grep-based check for orphaned getElementById targets and unused function declarations is cheap and high-signal here).
• Abstractions that only exist because of how the code evolved, not because the current shape needs them.
• Any place three similar things became a premature "generic" mechanism.

2. Security review.

Read these with intent to break them, not to confirm they're fine:

• /run (server.py:399) — takes ?cmd= as a GET query parameter, checks only cmd.startswith(p) against a small prefix whitelist (ALLOWED_PREFIXES), then runs it via subprocess.Popen(cmd, shell=True, ...). Everything after the allowed prefix is unvalidated and reaches a shell. Two angles worth checking specifically:
  • Is there a way to smuggle a second command past the prefix check (;, &&, backticks, $(...)) given shell=True?
  • It's a GET endpoint with no auth, bound to 127.0.0.1 — is it reachable via CSRF from a malicious page open in the same browser (DNS rebinding, or just an <img src="http://localhost:1312/run?cmd=..."> on any site the user visits while the server is running)? If so, the "same-machine only" assumption doesn't hold.
• Path handling in /library, /unimported, /playlists — do any of them accept a path that escapes the intended root via ../ or a symlink? /unimported in particular walks an arbitrary user-supplied directory with Path.rglob('*') and opens every matching file with mediafile.MediaFile() — check what happens on a symlink loop, a FIFO, a device file, or a deeply nested path that hits OS limits.
• POST /config writes arbitrary content to config.yaml after making a .bak copy — verify the .bak write can't be tricked into overwriting something outside the beets config directory, and that the write itself can't be used to inject config that gets eval'd or otherwise executed by beets on next load (check config['import']['log'], which is set from user-controlled log_path in importsession.start() — can it point somewhere unintended?).
• quality_rank() / resolve_duplicate in importsession.py — 'remove' deletes files from disk (task.should_remove_duplicates = True → ImportTask.remove_duplicates()). Confirm there's no path where a crafted duplicate scenario causes this to fire without an explicit human answer (re-read ImportJob.answer()'s validation in importsession.py — does it actually block every unsolicited choice, including replayed/duplicate decision_ids from a reconnecting SSE client?).

Acceptance

• Ponytail findings: each is either fixed or explicitly accepted with a ponytail: comment explaining the ceiling.
• Zero unresolved shell-injection or path-traversal findings, or each is explicitly accepted with a written reason (e.g. "not exploitable because X").
• The /run CSRF question above has a definitive answer, not a guess — either demonstrate the exploit against a live instance or show concretely why it can't work.
• Report is written findings, not a "looks fine" summary — every item in this issue gets an explicit line in the output, pass or fail.

Traps

• Don't confuse "runs on localhost" with "safe" — that assumption is exactly what needs testing, not accepting.
• /run's whitelist covers beet , fd , du , for — the for prefix in particular allows arbitrary shell loops; check what it's used for in beetsgui.html and whether that use is as narrow as intended.
• Don't just read the code and reason about it — where practical, actually run the exploit attempt against a live server.py instance (confined to a throwaway BEETSDIR, never the real one) and show the result.