cliamp: validate the path before checking the host for the binary
openPlayer checked `findCliamp()` before `resolveInHome(files)`, so a path escaping the owner's home was answered "cliamp not found on host" on any machine without the binary — telling the caller about the host in response to input that should simply have been refused. It also made the guarantee depend on an unrelated binary being installed, which is why cliamp-ws.test.ts's "rejects a path that escapes the owner home" failed on this machine (and every other one without cliamp) since the tests started running from plugins/ at all. It was being carried as a known-failing baseline rather than read. Validate the request, then inspect the host. No behavioural change on a machine that has cliamp. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
+10
-3
@@ -107,12 +107,19 @@ function pump(
|
||||
function openPlayer(ws: ServerWebSocket<MusicWSData>, files: string): void {
|
||||
if (!files) return sendOutput(ws, '\r\n[Error] No files specified.\r\n');
|
||||
|
||||
const cliampPath = findCliamp();
|
||||
if (!cliampPath) return sendOutput(ws, '\r\n[Error] cliamp not found on host.\r\n');
|
||||
|
||||
// Validate the REQUEST before inspecting the HOST, and in that order deliberately.
|
||||
//
|
||||
// It was the other way round until 2026-08-18, which had two costs. A path escaping the owner's home
|
||||
// was answered "cliamp not found on host" on a machine without the binary — telling the caller
|
||||
// something about the host in response to input that should simply have been refused. And the
|
||||
// behaviour this pair is supposed to guarantee became dependent on whether an unrelated binary happens
|
||||
// to be installed, which is what made `cliamp-ws.test.ts` fail on every machine that does not have it.
|
||||
const target = resolveInHome(files);
|
||||
if (!target) return sendOutput(ws, '\r\n[Error] Invalid file path.\r\n');
|
||||
|
||||
const cliampPath = findCliamp();
|
||||
if (!cliampPath) return sendOutput(ws, '\r\n[Error] cliamp not found on host.\r\n');
|
||||
|
||||
// `script` fakes a PTY for cliamp, which avoids a node-pty native dependency here.
|
||||
const cliampCmd = `${shellEscape(cliampPath)} ${shellEscape(target)}`;
|
||||
console.log(`[music] cliamp spawning: ${cliampCmd}`);
|
||||
|
||||
Reference in New Issue
Block a user