fix(subsonic): 修复 Subsonic 客户端无法播放 OpenList/WebDAV 挂载音乐 - #313
fix(subsonic): 修复 Subsonic 客户端无法播放 OpenList/WebDAV 挂载音乐#313boy6656598 wants to merge 40 commits into
Conversation
…tion & forced login - OpenList: multi-server CRUD config (openlist.json), browse/search/stream/lyric/upload via AList-compatible API, server-side proxy streaming with Range support - Alidrive: client config, QR login binding, file browse/play/upload/download - Cards: card-code registration for player accounts - Player: forced login, register page, OpenList & Alidrive manager modules - Admin: OpenList server management, Alidrive binding UI
- app.js: explicitly init/refresh AlidriveManager and OpenListManager in switchTab - Remove switchTab-overriding IIFEs from both manager scripts (they ran before app.js, dropping original tab logic) - docker-compose.yml: full deployment comments and env var examples (WEBPLAYER_PASSWORD/ENABLE_WEBPLAYER_AUTH) - scripts/migrate-to-nas.sh: package config.js + data/ into a tarball with NAS-side install steps
- stream 代理改用原生 http/https 替代 needle（needle 3.x 响应约 130KB 后卡死） - 新增边播边缓存：首次播放写入本地磁盘，后续播放/拖拽秒开 - 新增缓存接口：cache/check、cache/status、cache/clear（管理员） - 前端 OpenList 列表显示已缓存/缓存中徽标 - config.js 解除 git 跟踪并加入 .gitignore，防止真实凭据误提交
- openlist.ts: 递归扫描目录树生成本地音乐索引（含超时/目录数/并发防护），新增 /api/openlist/local-list 接口 - server.ts: /api/music/cache/list 合并 folder='openlist' 条目；stream 代理服务端跟随 302 重定向（上游直链跳转 OSS/CDN） - local_music.js: 本地音乐 tab 支持 openlist 筛选、播放/下载/收藏、目录加歌单、内嵌 OpenList 目录树浏览面板 - app.js: cleanSongData/formatSongToLxMusicStandard 保留 openlist 字段，收藏后恢复播放 - 新增 user.enableOpenListInLocalMusic 配置开关（默认开启）
- 部署方式重排: NAS Docker Compose 一键部署置顶, 补充 migrate-to-nas.sh 迁移脚本用法与 ./data 备份说明 - 新增群晖 SPK 套件安装指引与 Subsonic 访问地址 - 链接归属由 XCQ0607/lxserver 全部替换为 boy6656598/lxserver - 删除不存在的 star history 图表与 docs 外链
alist.embyfd.cc.cd 等域名同时解析出 IPv6(AAAA), Node 默认优先 IPv6,
而部分部署环境 IPv6 路由不可达导致 connect ENETUNREACH。在入口设置
dns.setDefaultResultOrder('ipv4first') 覆盖 needle/webdav/原生 http 全部出站请求
- 移除阿里云盘功能：删除 alidrive.ts/aliyun_manager.js、全部 /api/alidrive/* 路由及前后端 UI 引用 - 修复本地音乐播放：serveCacheFile 增加 Range 416 校验与 suffix range 支持，避免 seek 时崩溃 - 修复 OpenList/WebDAV 内网地址挂载：无协议地址默认补 http:// 而非强制 https
…-playback feat: remove alidrive & fix local playback & intranet mount
- 新增 src/server/webdavMount.ts: 多挂载源 CRUD 持久化(webdav-mounts.json)、 目录扫描音频索引(TTL缓存/防护)、边播边缓存(streamToCache .tmp->rename)、 本地Range服务/缓存进度/清空 - server.ts 新增 /api/webdav-mounts 系列路由(CRUD/test/available/browse/ local-list/stream/cache-check/status/clear); 主音乐列表合并 webdav 索引 - subsonic.ts handleStream 扩展: webdav_/openlist_/local 走内部流 302 - 前端: 后台 WebDAV 挂载管理视图(index.html/app.js); 播放器 WebDAV 目录树 面板与目录加歌单(local_music.js/index.html), 缓存进度徽标 - 单元测试 webdavMount.test.ts 20 项(node:test + mock WebDAV) - 更新 tasklist 标记全部任务完成
- webdavSync.scanFiles 排除 openlist-cache/webdav-cache 缓存目录，防止音频缓存同步到 dav 后被挂载源索引为远程音乐（自我污染） - openlist/webdav 索引扫描跳过 lx-sync/lx-sync-backups 同步目录 - webdavMount.joinRemotePath 对已含 rootPath 前缀的路径去重，修复 browse 路径翻倍导致目录树不显示文件夹 - webdavMount 索引 path 统一为相对 baseUrl 的完整路径（含 rootPath），修复主列表 stream 播放路径错误 - local_music playItem 改用 buildPlaylistSong 构造播放列表，保留 source/name/singer/url，修复 Invalid songInfo 与 Unknown 歌曲名 - webdav 播放 URL 追加 token 供 audio 鉴权
- 前端 fetchSongUrl 本地直放条件加入 /api/webdav-mounts/stream 前缀 - 前端 fetchLyric 新增 WebDAV 同目录 .lrc 歌词分支(经 /api/webdav-mounts/lyric) - 服务端 /api/music/url 对 webdav/openlist 兜底返回 stream URL, 避免 Unknown/自定义源错误 - 服务端 /api/music/lyric 对 webdav/openlist 直接返回空, 避免 Source not supported 刷屏 - webdavMount 新增 getLyric(同目录 .lrc), 配套 /api/webdav-mounts/lyric 路由与测试
- formatSongToLxMusicStandard: 新增 webdav case, 保留 url/serverId/path/webdav/folder, 与 openlist 对称 - cleanSongData: WebDAV 歌曲保留 serverId/path 并标记 webdav/isLocal - /api/music/url: webdav/openlist 兜底升级为自动恢复——path 可从 id 前缀解码, serverId 缺失时按 rootPath 前缀或本地索引自动查找挂载/服务器, 旧收藏(已丢失字段)也能恢复播放
…ices (Alist/WebDAV) from bridge container
- serveCacheFile 对 0 字节缓存返回无效并回退上游 dav 流式，避免 audio 收到空响应 - 修复下载/边播边缓存在上游无 Content-Length(total=0) 时无条件落盘的问题 - stream 转发 Content-Type 兜底：octet-stream/缺失时按扩展名映射正确音频 MIME - stream 支持 nocache=1 参数；前端 webdav 播放重试时强制绕过损坏缓存重新拉流
Subsonic 客户端仅用 u/p 参数认证，无 lxserver session cookie， stream 302 到 /api/openlist/stream、/api/webdav-mounts/stream 后返回 401。 在内部流 URL 附加 HMAC 签名令牌 sst（含 server+path+过期时间）， 两个 stream 端点校验放行，使箭头音乐等 Subsonic 客户端可播放挂载音乐。
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR adds OpenList and WebDAV music sources with browsing, indexing, streaming, caching, lyrics, playlists, Subsonic support, administration, card-based registration, forced login, deployment changes, and documentation. ChangesRemote music sources
Authentication and card management
Deployment and runtime
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (19)
Dockerfile-15-17 (1)
15-17: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake the migrated
config.jsreachable by the container.Line 17 removes
config.jsfrom the runtime image. Compose mounts only./data. The migration archive includes a cleanedconfig.js, but the container cannot read it. Migrated configuration is therefore ignored, or startup fails if the configuration loader requires/server/config.js.
Dockerfile#L15-L17: retain a default runtime configuration or move configuration loading to a persisted location.docker-compose.yml#L32-L34: mount the migrated configuration, or configureCONFIG_PATHto a mounted path.scripts/migrate-to-nas.sh#L18-L53: package configuration only at the path that the container loads.README.md#L174-L200: document the selected configuration mount and path.README_EN.md#L165-L191: document the selected configuration mount and path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` around lines 15 - 17, Make the migrated config.js available at the path consumed by the container: in Dockerfile lines 15-17, retain or relocate the runtime configuration; in docker-compose.yml lines 32-34, mount the migrated configuration or set CONFIG_PATH to its mounted location; in scripts/migrate-to-nas.sh lines 18-53, package config.js at that same path; and document the selected mount and configuration path in README.md lines 174-200 and README_EN.md lines 165-191. Ensure all five files use one consistent runtime configuration location.public/music/login.html-227-231 (1)
227-231: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftDo not persist passwords or bearer tokens in
localStorage.Any same-origin script can read
lx_sync_passandlx_user_token. This includes injected scripts. The password remains on the device after the browser closes.
public/music/login.html#L227-L231: remove persistent storage ofpasswordand migrate token consumers to the authenticated session flow.public/music/login.html#L278-L282: apply the same change after registration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/music/login.html` around lines 227 - 231, Remove persistence of lx_sync_pass and lx_user_token from both login.html sites at lines 227-231 and 278-282, covering the success paths after login and registration. Migrate any token consumers to the existing authenticated session flow while preserving username and mode storage as appropriate.Source: Linters/SAST tools
src/server/cards.ts-22-39 (1)
22-39: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winFail closed when card-state persistence fails.
loadCardsconverts an unreadable file into an empty card set.saveCardssuppresses write failures. IfconsumeCardcannot persist the used state, registration can still create the user. A later request reloads the unused card from disk and accepts it again.Throw on read or write errors. Do not replace invalid state with
[]. Write to a temporary file and rename it atomically.Proposed minimum fix
const saveCards = (): void => { try { fs.writeFileSync(cardsPath(), JSON.stringify(cards, null, 2), 'utf8') } catch (e) { console.error('[Cards] Failed to save cards:', e) + throw new Error('Failed to persist card state') } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/cards.ts` around lines 22 - 39, Update loadCards and saveCards to fail closed: propagate read, parse, and write errors instead of replacing invalid or unreadable state with [] or suppressing failures. In saveCards, write the serialized cards to a temporary file and atomically rename it to cardsPath() only after the write succeeds, preserving card state when persistence fails.src/server/server.ts-4448-4454 (1)
4448-4454: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not treat the username
adminas administrator authorization.
requirePlayerOrAdminreturns the string'admin'for a verified administrator password, but it also returns the raw username fromverifyUserAuthfor a normal account. The cache-clear route at Line 4930 comparesusername !== 'admin'. A registered user whose name isadmintherefore passes the administrator check. The registration validator at Line 4335 acceptsadminas a username.Use
requireAdminAuth()directly for this route.🔒️ Proposed fix
- const username = requirePlayerOrAdmin() - if (!username || username !== 'admin') { + if (!requireAdminAuth()) { res.writeHead(403, { 'Content-Type': 'application/json' }) res.end(JSON.stringify({ success: false, message: '需要管理员权限' })) return }Also applies to: 4928-4934
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/server.ts` around lines 4448 - 4454, Update the cache-clear route authorization to call requireAdminAuth() directly instead of relying on requirePlayerOrAdmin() and comparing its returned username to 'admin'. Preserve the existing denial behavior while ensuring a registered account named 'admin' cannot satisfy the administrator check.public/music/js/openlist_manager.js-260-266 (1)
260-266: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe badge refresh appends a duplicate list instead of updating it.
refreshCacheBadgescallsthis.renderList(false)at Line 295. InrenderList,reset === falsetakes theinsertAdjacentHTML('beforeend', html)branch at Line 263.htmlcontains the complete folder and audio list, so the whole directory listing is appended a second time. The user sees every entry twice after the first cache badge resolves.Re-render in place, or update only the badge elements by id, as
refreshWmCacheBadgesdoes inpublic/music/js/local_music.js.🐛 Proposed fix
if (JSON.stringify(changed) !== JSON.stringify(this._cacheBadges)) { this._cacheBadges = changed; const listEl = document.getElementById('ol-file-list'); - if (listEl && !this.loading) this.renderList(false); + if (listEl && !this.loading) this.renderList(true); }
renderList(true)overwritesinnerHTMLand keeps the list correct. Note that it callsrefreshCacheBadgesagain; the second pass produces an equalchangedobject and stops.Also applies to: 292-296
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/music/js/openlist_manager.js` around lines 260 - 266, Update refreshCacheBadges to call renderList(true) instead of renderList(false), ensuring the complete listing replaces the existing content rather than being appended as duplicates. Preserve the existing badge refresh flow and its changed-state termination behavior.src/server/server.ts-4326-4361 (1)
4326-4361: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winCheck the registration switch before you disclose account existence, and consume the card after the account is created.
Two ordering problems exist:
- Line 4345 returns 409 for an existing username before Line 4350 checks
player.enableRegister. An anonymous caller can enumerate accounts even when registration is closed. Move theenableRegistercheck to the top of the handler.cards.consumeCardruns at Line 4356 beforecheckAndCreateDirandsaveUsers. If directory creation or user persistence throws, the outercatchreturns 400 and the card stays consumed. Consume the card only after the user record is saved, or release it in the failure path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/server.ts` around lines 4326 - 4361, In the /api/auth/register POST handler, move the player.enableRegister check before username existence validation so closed registration never reveals whether an account exists. Reorder cards.consumeCard to run only after checkAndCreateDir and saveUsers successfully persist the new user, preserving the existing card-error response behavior.src/server/server.ts-4459-4475 (1)
4459-4475: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not fall back to a hardcoded HMAC secret.
Line 4468 uses the literal
'lxserver-internal'whenfrontend.passwordis empty. The literal is public in the repository. Any caller can then mint a validssttoken for anyserverandpathand stream every mounted file without a session. Generate a random per-process secret, or persist a dedicated stream secret, and rejectsstwhen no secret is configured.The token also binds only
server,path, and the expiry. It does not bind the requesting user, so a leaked stream URL is replayable until expiry. Keep the expiry window short.🔒️ Proposed fix
+ // 模块级：进程启动时生成随机内部签名密钥 + // const INTERNAL_STREAM_SECRET = global.lx.config['frontend.password'] || crypto.randomBytes(32).toString('hex') - const secret = global.lx.config['frontend.password'] || 'lxserver-internal' + const secret = INTERNAL_STREAM_SECRET🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/server.ts` around lines 4459 - 4475, Update verifyInternalStreamToken to eliminate the hardcoded fallback secret: use a securely generated per-process or persisted dedicated stream secret, and reject sst tokens when no secret is configured. Bind tokens to the requesting user where available and enforce a short expiry window, preserving constant-time signature validation.public/music/js/local_music.js-49-56 (1)
49-56: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDeclare the WebDAV panel state next to the OpenList panel state.
The block declares
olServers,olCurrentPath,olBreadcrumb,olItems,olSearchMode, andolPanelInitialized. The WebDAV counterpartswmServers,wmCurrentServerId,wmCurrentPath,wmBreadcrumb,wmItems, andwmPanelInitializedare never declared. They stayundefineduntil the first assignment.
wmAddCurrentDirToPlaylist(Line 1367) andwmPlayAudio(Line 1342) callthis.wmItems.filter(...). Both buttons exist in the DOM as soon as the panel opens. If the user opens the WebDAV panel and clicks "目录加歌单" before any mount is selected,this.wmItemsisundefinedand the call throws a TypeError.Also declare
olSearchKeyword, whichloadOlListreads at Line 876.🐛 Proposed fix
olItems: [], olSearchMode: false, + olSearchKeyword: '', olPanelInitialized: false, + // [新增] 内嵌 WebDAV 挂载目录树浏览面板 + wmServers: [], + wmCurrentServerId: '', + wmCurrentPath: '/', + wmBreadcrumb: [], + wmItems: [], + wmPanelInitialized: false,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/music/js/local_music.js` around lines 49 - 56, Declare the missing WebDAV panel state alongside the existing OpenList state: initialize wmServers, wmCurrentServerId, wmCurrentPath, wmBreadcrumb, wmItems, and wmPanelInitialized with appropriate empty defaults so wmAddCurrentDirToPlaylist and wmPlayAudio can safely call wmItems.filter before mount selection. Also initialize olSearchKeyword for loadOlList to read.src/server/server.ts-4794-4807 (1)
4794-4807: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winApply the same zero-byte guard that the WebDAV path uses.
The WebDAV stream handler at Line 5287 requires
cacheReceived > 0before it renames the temporary file. The OpenList handler here does not. When the upstream returns an empty body withoutContent-Length,totalis0andcacheReceivedis0, so a zero-byte file is promoted to the cache. Every later request then hits that broken cache entry throughserveCacheFileat Line 4732.🐛 Proposed fix
- if (total === 0 || cacheReceived >= total) { + if (cacheReceived > 0 && (total === 0 || cacheReceived >= total)) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/server.ts` around lines 4794 - 4807, Update the completion condition in the OpenList response end handler around cacheWs.end so temporary files are renamed only when cacheReceived is greater than zero and the existing total-length condition is satisfied. Preserve the current cleanup and openlist.clearCache behavior for zero-byte or incomplete downloads, matching the guard used by the WebDAV stream handler.src/server/server.ts-5563-5567 (1)
5563-5567: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winRemove the synchronous debug file writes from the search route.
Every search request and every search failure calls
fs.appendFileSyncondebug.txtin the process working directory. The call blocks the event loop on the request path, and the file grows without any rotation or size limit. Search queries are user input, so this also persists user activity to an unmanaged file.Use the existing logger instead.
♻️ Proposed fix
- fs.appendFileSync(path.join(process.cwd(), 'debug.txt'), `[Search] Source: ${source}, Type: ${type}, Query: ${name}, StartPage: ${page}, Pages: ${fetchPages}, Result Count: ${result.length}\n`) res.writeHead(200, { 'Content-Type': 'application/json' }) res.end(JSON.stringify(result)) } catch (err: any) { - fs.appendFileSync(path.join(process.cwd(), 'debug.txt'), `[Search Error] ${err.message}\n${err.stack}\n`) - console.error(err) + accessLog.warn(`[Search] failed source=${source} type=${type}: ${err.message}`)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/server.ts` around lines 5563 - 5567, Remove both synchronous fs.appendFileSync calls from the search route’s success and catch paths, and replace them with the existing logger while preserving the search result and error-handling behavior. Use the logger to record the search context and failure details without writing user queries to debug.txt.public/music/js/openlist_manager.js-320-325 (1)
320-325: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the parent directory when resolving OpenList search results.
_fullPathcurrently prefixes every search result with/, so playback, download, and cache-check requests omit the file’s actual directory. Pass each audio item into_fullPath()and build the path fromitem.parentwhere available so the resolved paths match the search response.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@public/music/js/openlist_manager.js` around lines 320 - 325, Update _fullPath and its callers so search results resolve against each audio item’s parent directory instead of always using the root path. Pass the relevant item into _fullPath for playback, download, and cache-check requests, use item.parent when available, and preserve currentPath behavior for non-search paths.src/server/openlist.ts-478-484 (1)
478-484: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winA rejected index scan is cached permanently in both remote-source modules.
getLocalIndexstores the in-flight scan promise inlocalIndexCache[id].pendingand never removes it when the scan rejects. Later non-forced calls return that same rejected promise, so the index never recovers without a process restart or an explicitclearLocalIndex. BecausegetAllLocalIndexusesPromise.all, one poisoned entry rejects the merged index for every configured source.src/server/subsonic.tsresolveLocalStreamUrlawaitsgetAllLocalIndex, so Subsonic playback of all remote tracks fails from that point on.
src/server/openlist.ts#L478-L484: add a.catchto thecollectAudioFiles(...).then(...)chain that deleteslocalIndexCache[serverId].pendingwhen it still references this promise, logs the error, and resolves to an empty array.src/server/webdavMount.ts#L282-L288: apply the identical.catchto thecollectAudioFiles(mount, '/')chain, clearinglocalIndexCache[mountId].pending.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/openlist.ts` around lines 478 - 484, The in-flight index scan promises remain cached after rejection, permanently poisoning remote-source lookups. In src/server/openlist.ts#L478-L484, update the getLocalIndex collection chain around collectAudioFiles to catch failures, delete localIndexCache[serverId].pending only when it still references that promise, log the error, and resolve with an empty array; apply the identical cleanup and fallback in src/server/webdavMount.ts#L282-L288 for localIndexCache[mountId].pending.src/server/subsonic.ts-584-584 (1)
584-584: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftA cold index cache makes the Subsonic stream request block on a full remote scan.
getAllLocalIndexreturns the cached index only inside the 120s TTL. On a miss it callscollectAudioFiles, which recursively walks the remote tree. The guards insrc/server/webdavMount.tsallow up toMAX_SCAN_MS = 60 * 1000, 800 directories, and 5000 files, and each directory listing has its own 20s timeout.
handleStreamawaits this before it can send the 302. A Subsonic client that requests the first track after the TTL expires waits for the whole scan. Most clients abort long before 60 seconds, so playback fails intermittently after idle periods.Consider serving a stale index while a refresh runs in the background, or resolve the path directly from the
songmidwithout requiring the full index.Also applies to: 602-602
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/subsonic.ts` at line 584, The handleStream flow should not await a cold getAllLocalIndex scan before issuing the stream redirect. Change the index lookup around getAllLocalIndex to serve the existing stale cache while triggering any refresh asynchronously, or resolve the requested path directly from songmid; preserve normal cached lookups while ensuring the first-track request is not blocked by collectAudioFiles.src/server/webdavMount.ts-396-409 (1)
396-409: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSet a socket timeout on the WebDAV stream request.
lib.requestis created without a timeout, and Line 406 swallows errors. If the WebDAV host accepts the TCP connection and then stops responding, noresponseand noerrorevent fires. The request hangs for the process lifetime.The consequence is not limited to one request.
downloadToCache(Line 464) wraps this request in a promise that resolves only from aresponseorerrorhandler. A hung request means that promise never settles, andinFlight[key](Line 528) keeps it forever. Every laterdownloadToCachecall for the same file returns the same never-settling promise.🐛 Proposed fix
export const stream = (mount: WebDAVMount, filePath: string, range?: string): http.ClientRequest => { const targetUrl = new URL(fileUrl(mount, filePath)) const headers: Record<string, string> = { 'User-Agent': 'lxserver/1.0' } if (mount.username && mount.password) { const token = Buffer.from(`${mount.username}:${mount.password}`).toString('base64') headers['Authorization'] = `Basic ${token}` } if (range) headers['Range'] = range const lib = targetUrl.protocol === 'https:' ? https : http - const req = lib.request(targetUrl, { method: 'GET', headers } as any) + const req = lib.request(targetUrl, { method: 'GET', headers, timeout: 30000 } as any) + // 无响应时主动销毁，触发 'error'，避免调用方 Promise 永不 settle + req.on('timeout', () => { req.destroy(new Error('WebDAV stream timed out')) }) req.on('error', () => { /* 错误由调用方处理 */ }) req.end() return req }Note that
req.destroy(err)emitserror, whichdownloadToCachealready handles at Line 480.Apply the same timeout to
openlist.streaminsrc/server/openlist.tsLine 270.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/webdavMount.ts` around lines 396 - 409, Set a socket timeout on the requests created by both WebDAV stream and openlist.stream, and destroy the request with an error when the timeout fires so the existing downloadToCache error handler settles its promise. Preserve the current request setup and error propagation while ensuring stalled connections cannot remain pending indefinitely.src/server/subsonic.ts-2055-2067 (1)
2055-2067: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winThe
ssttoken travels in a query string and will be written to access logs.Line 2060 puts the signed token in the
Locationquery string. The redirected request then reaches the main dispatcher insrc/server/server.ts, which logs the full URL throughaccessLog.info(\${req.method} ${req.url} from ${ip}`)`. The token that authorizes file access is therefore persisted in plaintext logs, and it stays valid for the remaining part of its 600-second window. Reverse proxies and client-side logs capture it as well.Consider reducing the validity window, and redacting the
sstparameter before logging.🛡️ Suggested log redaction
- accessLog.info(`${req.method} ${req.url} from ${ip}`) + const safeUrl = (req.url ?? '').replace(/([?&](?:sst|sign)=)[^&]*/gi, '$1<redacted>') + accessLog.info(`${req.method} ${safeUrl} from ${ip}`)Apply this in
src/server/server.tsat the dispatcher shown in the provided context.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/subsonic.ts` around lines 2055 - 2067, Redact the sst query parameter before the dispatcher access log in server.ts records req.url, while preserving other URL components and existing access-log behavior. Update the relevant request-logging flow to log a sanitized URL, and reduce the signed-token validity window from its current 600 seconds where that token lifetime is configured.src/server/subsonic.ts-615-626 (1)
615-626: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPoint the local cache lookup at the actual FileCache base.
getUserDirname(username)only returns the user directory name, socacheRootis built underprocess.cwd()instead of the configured cache root. For local streams, build the candidate cache root with the same logic used byserveCacheFile: check the active cache location under bothcacheandmusic, or import and callsetCacheLocationinstead of hard-codingmusic/cache. As written, local-cache Subsonic playback returnsCould not resolve local track.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/subsonic.ts` around lines 615 - 626, Update the local cache lookup around getUserDirname and cacheRoot to resolve the configured FileCache base using the same cache-location logic as serveCacheFile, checking active cache paths under both cache and music as applicable. Do not derive the root solely from process.cwd() plus music/cache; reuse setCacheLocation or the established cache-location helper so local Subsonic streams locate cached tracks correctly.src/server/webdavMount.ts-71-84 (1)
71-84: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winValidate the caller-supplied mount
idbefore it reaches filesystem paths.Line 75 accepts
data.idfrom the caller and only generates an id when it is absent.addMountis reachable from the mount-creation API, so the storedidcan contain path separators or... Thatidis then joined into filesystem paths without normalization:
- Line 112 / Line 113:
fs.rmSync(path.join(dataPath, 'webdav-cache', id), { recursive: true, force: true })on delete.- Line 316:
getCacheDircreates the directory.- Line 561:
clearCacheremoves it.An id such as
../../..turns mount deletion into an arbitrary recursive directory removal. The operation requires administrator access, but the guarantee should not depend on that. Restrict the id to a safe character set.🛡️ Proposed fix
+const SAFE_ID_RE = /^[A-Za-z0-9_-]{1,64}$/ + const normalizeMount = (data: any): WebDAVMount => { let baseUrl = String(data.baseUrl || '').trim().replace(/\/+$/, '') if (baseUrl && !/^https?:\/\//i.test(baseUrl)) baseUrl = 'http://' + baseUrl + const rawId = String(data.id || '') + if (rawId && !SAFE_ID_RE.test(rawId)) throw new Error('非法的挂载源 ID') return { - id: data.id || 'wd_' + crypto.randomBytes(6).toString('hex'), + id: rawId || 'wd_' + crypto.randomBytes(6).toString('hex'),Apply the same validation to the
idused bygetCacheDir,clearCache, andcacheStatusif those can be reached with an id that was not loaded from the config file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/webdavMount.ts` around lines 71 - 84, Validate mount IDs in normalizeMount and in getCacheDir, clearCache, and cacheStatus before using them in filesystem paths, restricting them to a safe character set that excludes path separators, traversal components, and other unsafe input. Preserve valid configured/generated IDs, and reject or safely handle invalid caller-supplied IDs consistently across creation and cache operations.src/utils/webdavSync.ts-209-234 (1)
209-234: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftDo not let incomplete listings drive restore consistency uploads.
listRemoteFilestreats skipped deep subtrees and failed listings as empty subtrees, whilerestoreFromRemoteuses the resultingremoteFileSetfor the “missing on cloud” upload step. Partial listings can omit valid remote files, so local copies get re-uploaded and the restore reports success without the missing files.Make
listRemoteFilesdistinguish depth truncation and listing failures from empty directories, and skip the consistency-upload step when the listing is incomplete.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/webdavSync.ts` around lines 209 - 234, Update listRemoteFiles to propagate an explicit incomplete-listing state when LIST_DEPTH_LIMIT truncates recursion or getDirectoryContents fails, while preserving empty results for genuinely empty directories. In restoreFromRemote, detect that state and skip the “missing on cloud” consistency-upload step; only perform remoteFileSet-based uploads after a complete listing.src/server/webdavMount.ts-149-166 (1)
149-166: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDistinguish a listing timeout from an empty directory.
Promise.raceresolves[]when the timeout fires, solistFilesreturns{ items: [] }with noerror. Callers then treat an unresponsive WebDAV directory as valid, with false-positive connection results and silent index generation that can cache a partial result. Abort the pending request instead of leaving it running after the timeout.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/webdavMount.ts` around lines 149 - 166, Update listFiles so a timeout from Promise.race is distinguished from a genuinely empty directory: reject or throw a timeout error when timeoutMs elapses, abort/cancel the pending getDirectoryContents request, and let the existing catch return an error alongside empty items. Preserve successful empty-directory results as { items: [] }.
🟡 Minor comments (4)
README.md-181-190 (1)
181-190: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMake migration archive extraction conditional.
A fresh installation does not have
lxserver-nas-deploy.tar.gz. The documented command fails before Compose starts. Split the migration extraction steps from the fresh-install steps.
README.md#L181-L190: runtar -xzfonly for migration installs.README_EN.md#L172-L181: runtar -xzfonly for migration installs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 181 - 190, Update the installation instructions in README.md lines 181-190 and README_EN.md lines 172-181 so fresh-install steps do not run tar extraction for the migration archive. Separate the migration-only extraction command from the shared directory setup and Docker Compose build/start steps, preserving equivalent guidance in both languages.src/server/server.ts-5314-5317 (1)
5314-5317: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClient disconnect does not cancel the in-flight redirect request. Both stream handlers register
req.on('close')against the initialproxyReqonly. AfterfollowRedirectswitches tonextReq, a client disconnect leaves the redirected upstream download running and consuming bandwidth. Track the current upstream request in a mutable variable and destroy that request on close.
src/server/server.ts#L5314-L5317: store eachnextReqin a shared variable inside the WebDAVfollowRedirectand destroy it in theclosehandler.src/server/server.ts#L4826-L4828: apply the same change in the OpenListfollowRedirect.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/server.ts` around lines 5314 - 5317, Update both followRedirect implementations in src/server/server.ts at lines 5314-5317 and 4826-4828: maintain a shared mutable reference to the current upstream request, assign each redirected nextReq to it, and have the req close handler destroy that current request rather than only the initial proxyReq.src/server/hostResolver.ts-15-21 (1)
15-21: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSkip comment lines in
parseHostsFile.The function does not strip
#comments. For a line such as# host.docker.internal 192.168.1.10,partsis['#', 'host.docker.internal', '192.168.1.10']. The condition at Line 18 matches and the function returns'#'.resolveHostthen builds a URL likehttp://#:5244, which fails with a confusing error rather than falling through to the gateway probe.🛡️ Proposed fix
export const parseHostsFile = (content: string): string => { for (const line of content.split('\n')) { - const parts = line.trim().split(/\s+/) - if (parts.includes('host.docker.internal') && parts[0]) return parts[0] + const stripped = line.split('#')[0].trim() + if (!stripped) continue + const parts = stripped.split(/\s+/) + if (parts.includes('host.docker.internal') && parts[0] !== 'host.docker.internal') return parts[0] } return '' }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/hostResolver.ts` around lines 15 - 21, Update parseHostsFile to skip lines whose trimmed content begins with # before splitting and matching host.docker.internal, ensuring commented entries cannot be returned as hostnames and valid entries still use the existing resolution flow.src/server/openlist.ts-308-316 (1)
308-316: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winApply the 0-byte cache guard to
openlist.serveCacheFile.
webdavMount.serveCacheFilereturnsfalsefor a 0-byte cache file so the caller falls back to the upstream stream. This function does not. If an OpenList cache file is left at 0 bytes by an interrupted download, this function responds200withContent-Length: 0and playback fails with no recovery until the cache is cleared. The two functions serve the same role, so they should apply the same defense.🐛 Proposed fix
export const serveCacheFile = (filePath: string, range: string | undefined, res: any): boolean => { if (!fs.existsSync(filePath)) return false const stat = fs.statSync(filePath) + // 损坏缓存防御：0 字节文件视为无效缓存，回退到上游流式 + if (stat.size === 0) return false const ext = path.extname(filePath).toLowerCase()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/openlist.ts` around lines 308 - 316, Update serveCacheFile to return false when the cached file’s stat.size is 0, before constructing or sending the response, so callers fall back to the upstream stream while preserving normal handling for non-empty cache files.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/server/server.ts`:
- Around line 1863-1872: Update the login handler around the loginHeaders
construction at src/server/server.ts:1863-1872 to widen its header type to
Record<string, string | string[]> and assign the cookie array directly to
Set-Cookie instead of joining it. Apply the same change to the registration
handler around src/server/server.ts:4374-4382 for regHeaders, preserving both
individual cookie values.
In `@src/server/subsonic.ts`:
- Around line 560-566: Update signInternalStream to use a dedicated
startup-initialized internal stream secret instead of
global.lx.config['frontend.password'] or any hard-coded fallback. Persist and
restore this secret with server state, fail closed when it is uninitialized, and
canonicalize serverId/filePath/expiry so delimiters such as | cannot create
ambiguous payloads; ensure the corresponding verifier in the server flow uses
the same secret and canonical payload.
In `@src/server/webdavMount.ts`:
- Around line 497-525: Replace the manual data handling in the response download
flow with resp.pipe(ws) to apply backpressure, and handle completion via the
write stream’s finish event instead of resp.on('end')/ws.end(). Add an explicit
ws.on('error') handler that destroys or cleans up the response and temporary
file, then resolves the operation as failed; retain the existing successful
rename and cache-finalization logic in the finish path.
---
Major comments:
In `@Dockerfile`:
- Around line 15-17: Make the migrated config.js available at the path consumed
by the container: in Dockerfile lines 15-17, retain or relocate the runtime
configuration; in docker-compose.yml lines 32-34, mount the migrated
configuration or set CONFIG_PATH to its mounted location; in
scripts/migrate-to-nas.sh lines 18-53, package config.js at that same path; and
document the selected mount and configuration path in README.md lines 174-200
and README_EN.md lines 165-191. Ensure all five files use one consistent runtime
configuration location.
In `@public/music/js/local_music.js`:
- Around line 49-56: Declare the missing WebDAV panel state alongside the
existing OpenList state: initialize wmServers, wmCurrentServerId, wmCurrentPath,
wmBreadcrumb, wmItems, and wmPanelInitialized with appropriate empty defaults so
wmAddCurrentDirToPlaylist and wmPlayAudio can safely call wmItems.filter before
mount selection. Also initialize olSearchKeyword for loadOlList to read.
In `@public/music/js/openlist_manager.js`:
- Around line 260-266: Update refreshCacheBadges to call renderList(true)
instead of renderList(false), ensuring the complete listing replaces the
existing content rather than being appended as duplicates. Preserve the existing
badge refresh flow and its changed-state termination behavior.
- Around line 320-325: Update _fullPath and its callers so search results
resolve against each audio item’s parent directory instead of always using the
root path. Pass the relevant item into _fullPath for playback, download, and
cache-check requests, use item.parent when available, and preserve currentPath
behavior for non-search paths.
In `@public/music/login.html`:
- Around line 227-231: Remove persistence of lx_sync_pass and lx_user_token from
both login.html sites at lines 227-231 and 278-282, covering the success paths
after login and registration. Migrate any token consumers to the existing
authenticated session flow while preserving username and mode storage as
appropriate.
In `@src/server/cards.ts`:
- Around line 22-39: Update loadCards and saveCards to fail closed: propagate
read, parse, and write errors instead of replacing invalid or unreadable state
with [] or suppressing failures. In saveCards, write the serialized cards to a
temporary file and atomically rename it to cardsPath() only after the write
succeeds, preserving card state when persistence fails.
In `@src/server/openlist.ts`:
- Around line 478-484: The in-flight index scan promises remain cached after
rejection, permanently poisoning remote-source lookups. In
src/server/openlist.ts#L478-L484, update the getLocalIndex collection chain
around collectAudioFiles to catch failures, delete
localIndexCache[serverId].pending only when it still references that promise,
log the error, and resolve with an empty array; apply the identical cleanup and
fallback in src/server/webdavMount.ts#L282-L288 for
localIndexCache[mountId].pending.
In `@src/server/server.ts`:
- Around line 4448-4454: Update the cache-clear route authorization to call
requireAdminAuth() directly instead of relying on requirePlayerOrAdmin() and
comparing its returned username to 'admin'. Preserve the existing denial
behavior while ensuring a registered account named 'admin' cannot satisfy the
administrator check.
- Around line 4326-4361: In the /api/auth/register POST handler, move the
player.enableRegister check before username existence validation so closed
registration never reveals whether an account exists. Reorder cards.consumeCard
to run only after checkAndCreateDir and saveUsers successfully persist the new
user, preserving the existing card-error response behavior.
- Around line 4459-4475: Update verifyInternalStreamToken to eliminate the
hardcoded fallback secret: use a securely generated per-process or persisted
dedicated stream secret, and reject sst tokens when no secret is configured.
Bind tokens to the requesting user where available and enforce a short expiry
window, preserving constant-time signature validation.
- Around line 4794-4807: Update the completion condition in the OpenList
response end handler around cacheWs.end so temporary files are renamed only when
cacheReceived is greater than zero and the existing total-length condition is
satisfied. Preserve the current cleanup and openlist.clearCache behavior for
zero-byte or incomplete downloads, matching the guard used by the WebDAV stream
handler.
- Around line 5563-5567: Remove both synchronous fs.appendFileSync calls from
the search route’s success and catch paths, and replace them with the existing
logger while preserving the search result and error-handling behavior. Use the
logger to record the search context and failure details without writing user
queries to debug.txt.
In `@src/server/subsonic.ts`:
- Line 584: The handleStream flow should not await a cold getAllLocalIndex scan
before issuing the stream redirect. Change the index lookup around
getAllLocalIndex to serve the existing stale cache while triggering any refresh
asynchronously, or resolve the requested path directly from songmid; preserve
normal cached lookups while ensuring the first-track request is not blocked by
collectAudioFiles.
- Around line 2055-2067: Redact the sst query parameter before the dispatcher
access log in server.ts records req.url, while preserving other URL components
and existing access-log behavior. Update the relevant request-logging flow to
log a sanitized URL, and reduce the signed-token validity window from its
current 600 seconds where that token lifetime is configured.
- Around line 615-626: Update the local cache lookup around getUserDirname and
cacheRoot to resolve the configured FileCache base using the same cache-location
logic as serveCacheFile, checking active cache paths under both cache and music
as applicable. Do not derive the root solely from process.cwd() plus
music/cache; reuse setCacheLocation or the established cache-location helper so
local Subsonic streams locate cached tracks correctly.
In `@src/server/webdavMount.ts`:
- Around line 396-409: Set a socket timeout on the requests created by both
WebDAV stream and openlist.stream, and destroy the request with an error when
the timeout fires so the existing downloadToCache error handler settles its
promise. Preserve the current request setup and error propagation while ensuring
stalled connections cannot remain pending indefinitely.
- Around line 71-84: Validate mount IDs in normalizeMount and in getCacheDir,
clearCache, and cacheStatus before using them in filesystem paths, restricting
them to a safe character set that excludes path separators, traversal
components, and other unsafe input. Preserve valid configured/generated IDs, and
reject or safely handle invalid caller-supplied IDs consistently across creation
and cache operations.
- Around line 149-166: Update listFiles so a timeout from Promise.race is
distinguished from a genuinely empty directory: reject or throw a timeout error
when timeoutMs elapses, abort/cancel the pending getDirectoryContents request,
and let the existing catch return an error alongside empty items. Preserve
successful empty-directory results as { items: [] }.
In `@src/utils/webdavSync.ts`:
- Around line 209-234: Update listRemoteFiles to propagate an explicit
incomplete-listing state when LIST_DEPTH_LIMIT truncates recursion or
getDirectoryContents fails, while preserving empty results for genuinely empty
directories. In restoreFromRemote, detect that state and skip the “missing on
cloud” consistency-upload step; only perform remoteFileSet-based uploads after a
complete listing.
---
Minor comments:
In `@README.md`:
- Around line 181-190: Update the installation instructions in README.md lines
181-190 and README_EN.md lines 172-181 so fresh-install steps do not run tar
extraction for the migration archive. Separate the migration-only extraction
command from the shared directory setup and Docker Compose build/start steps,
preserving equivalent guidance in both languages.
In `@src/server/hostResolver.ts`:
- Around line 15-21: Update parseHostsFile to skip lines whose trimmed content
begins with # before splitting and matching host.docker.internal, ensuring
commented entries cannot be returned as hostnames and valid entries still use
the existing resolution flow.
In `@src/server/openlist.ts`:
- Around line 308-316: Update serveCacheFile to return false when the cached
file’s stat.size is 0, before constructing or sending the response, so callers
fall back to the upstream stream while preserving normal handling for non-empty
cache files.
In `@src/server/server.ts`:
- Around line 5314-5317: Update both followRedirect implementations in
src/server/server.ts at lines 5314-5317 and 4826-4828: maintain a shared mutable
reference to the current upstream request, assign each redirected nextReq to it,
and have the req close handler destroy that current request rather than only the
initial proxyReq.
---
Nitpick comments:
In @.monkeycode/specs/webdav-mount-cache/design.md:
- Around line 103-116: Update the route table to reflect the flat endpoints
implemented in server.ts: use /api/webdav-mounts with POST, PUT, and DELETE,
documenting that the id is supplied in the JSON body, and replace :id/test,
:id/browse, and :id/local-list with their query-parameter-based routes,
including browse as /api/webdav-mounts/browse?server=&path= and the
corresponding implemented parameters for testing and local-list.
In `@public/app.js`:
- Line 2022: Replace the native confirm() calls in all three new delete paths
with the shared showSelect(...) confirmation flow, matching the existing
implementations in deletePlaylist and deleteUser. Preserve each path’s current
deletion behavior and ensure deletion proceeds only after the shared dialog is
confirmed.
- Around line 2100-2102: Update the inline onclick handlers generated in the
list template to pass this.escapeHtml(s.id) instead of the raw s.id for
testOpenList, editOpenList, and deleteOpenList; apply the same escaping to m.id
in the corresponding handlers around the other referenced location, preserving
the existing method calls and template structure.
In `@public/music/app.js`:
- Around line 11993-12018: Update showInputModal to prevent interpolated title,
message, f.label, f.placeholder, and f.value from being inserted into innerHTML
unescaped. Apply consistent HTML escaping before interpolation, or construct the
input elements with DOM APIs and assign their value properties, while preserving
the existing modal structure and behavior.
- Around line 9831-9874: Remove the duplicated OpenList and WebDAV field
assignments from their switch cases, leaving those cases to set the id and any
necessary source selection. Keep the fallback blocks after the switch, widening
their conditions to include both the source flags and source values: s.openlist
|| source === 'openlist' and s.webdav || source === 'webdav'.
In `@public/music/js/local_music.js`:
- Around line 1291-1317: Update refreshWmCacheBadges to issue cache-check
fetches concurrently with Promise.all instead of awaiting each audio
sequentially, while preserving per-request error handling and badge generation.
Capture the directory/server context before requests begin and only apply
changed badges if that context still matches the current view, preventing stale
responses from overwriting badges after navigation.
- Around line 942-1009: Extract the duplicated directory-list rendering logic
from renderOlList and renderWmList into one parameterized helper, using a
per-source configuration for element IDs, directory-field name, and accent color
while preserving existing behavior. Reuse the helper for both source-specific
render methods, including shared audio filtering, folder/lyric partitioning,
empty state, navigation row, and append/reset handling. Apply the same
deduplication pattern to loadOlServers/loadWmServers, olGoTo/wmGoTo,
olGoBack/wmGoBack, and olBuildSong/wmBuildSong, keeping source-specific
differences in configuration rather than duplicated logic.
In `@public/music/js/openlist_manager.js`:
- Around line 229-234: Remove the unused index plumbing in the audio rendering
loop: update the audios.forEach callback and eliminate the globalIndex alias,
then change the playAudio call and its definition to omit the audioIndex
parameter while preserving the existing filename-based index lookup. Apply the
same call-site update to the additional location around the other audio list
rendering.
In `@src/server/openlist.ts`:
- Around line 130-132: Update the needle.request call to pass data directly
instead of using the redundant isDownload ? data : data expression, while
preserving the existing payload behavior for downloads and non-downloads.
- Around line 550-579: Update the download request in the surrounding try block
and the upload request options used by needle.put to use finite open_timeout,
response_timeout, and read_timeout values instead of timeout: 0. Apply the same
appropriate timeout configuration to both needle.get(sourceUrl, ...) and the PUT
options, preserving the existing streaming and error-handling behavior.
In `@src/server/server.ts`:
- Around line 5746-5810: The WebDAV/OpenList fallback in the server handler
performs an unbounded getLocalIndex scan across every candidate when serverId is
missing. Replace this per-request probing with a short-lived path-to-mount
cache, or resolve the mount exclusively from persisted song metadata, so
playback does not trigger multiple recursive remote scans; preserve the existing
stream URL response once a mount is resolved.
In `@src/server/webdavMount.test.ts`:
- Line 376: Remove the unused crypto import and the standalone void crypto
statement from the webdavMount test, leaving the existing tests unchanged.
- Around line 323-334: Extend the test around serveCacheFile to create a
zero-byte cache file, invoke wm.serveCacheFile with it, and assert it returns
false so the upstream fallback is exercised. Also add downloadToCache
rejection-path coverage for upstream 4xx, 3xx responses with Location, and empty
200 bodies, asserting each removes both the temporary file and final cache file.
- Around line 101-104: Move the mock-server startup from the first test into a
before hook for the webdavMount 边播边缓存 describe block, initializing the shared
port before any tests construct baseUrl. Remove the now-unnecessary mid-file
after import while preserving the existing server cleanup behavior.
- Around line 212-230: Strengthen the three tests around getLocalIndex,
getAllLocalIndex, and clearLocalIndex: track mock-server PROPFIND requests and
assert the second TTL-cached getLocalIndex call does not increase the count;
create distinct enabled mounts and assert getAllLocalIndex contains entries from
both indexes rather than merely being an array; after clearLocalIndex, assert
the subsequent getLocalIndex performs a fresh scan or otherwise differs from the
cached result to prove the cache entry was removed.
In `@src/server/webdavMount.ts`:
- Around line 535-553: Update cacheStatus and its recursive scan to avoid
synchronous filesystem operations that block the event loop: make them
asynchronous, use fs.promises.readdir with withFileTypes and fs.promises.stat,
await recursive scans and stats, and return the same { fileCount, size } result
while preserving mountId filtering and .tmp exclusion.
- Around line 124-130: Remove the unused force parameter from initClient and
update all callers, including listFiles and getLyric, to use its simplified
signature. Cache clients by mount id so repeated requests reuse the same WebDAV
client, and invalidate the corresponding cache entry in updateMount and
deleteMount.
In `@src/utils/webdavSync.ts`:
- Around line 35-42: Extract the duplicated normalizeWebdavUrl function into a
shared utility module, then remove the local definitions and import the shared
symbol in both webdavSync and webdavMount. Preserve the existing trimming,
protocol detection, and http:// fallback behavior exactly.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0f69d08a-979e-4a92-b186-a158c6d1047d
📒 Files selected for processing (33)
.github/workflows/docker.yml.gitignore.monkeycode/MEMORY.md.monkeycode/specs/webdav-mount-cache/design.md.monkeycode/specs/webdav-mount-cache/requirements.md.monkeycode/specs/webdav-mount-cache/tasklist.mdDockerfileREADME.mdREADME_EN.mdconfig.jsdocker-compose.ymlpublic/app.jspublic/index.htmlpublic/js/config.jspublic/music/app.jspublic/music/index.htmlpublic/music/js/local_music.jspublic/music/js/openlist_manager.jspublic/music/login.htmlscripts/migrate-to-nas.shsrc/defaultConfig.tssrc/index.tssrc/server/cards.tssrc/server/fileCache.tssrc/server/hostResolver.test.tssrc/server/hostResolver.tssrc/server/openlist.tssrc/server/server.tssrc/server/subsonic.tssrc/server/webdavMount.test.tssrc/server/webdavMount.tssrc/types/config.d.tssrc/utils/webdavSync.ts
💤 Files with no reviewable changes (1)
- config.js
| const loginHeaders: Record<string, string> = { 'Content-Type': 'application/json' } | ||
| if (global.lx.config['player.forceLogin']) { | ||
| const sessionId = generateSessionId() | ||
| playerSessions.set(sessionId, { createdAt: Date.now() }) | ||
| const cookies: string[] = [] | ||
| cookies.push(`${SESSION_COOKIE_NAME}=${sessionId}; HttpOnly; Path=/; SameSite=Strict; Max-Age=${SESSION_TTL / 1000}`) | ||
| cookies.push(`${USER_TOKEN_COOKIE_NAME}=${token}; Path=/; SameSite=Lax; Max-Age=${USER_SESSION_TTL / 1000}`) | ||
| loginHeaders['Set-Cookie'] = cookies.join(', ') | ||
| } | ||
| res.writeHead(200, loginHeaders) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Comma-joined Set-Cookie header in the login and registration handlers. Both handlers build two cookies and join them with , into one header value. Browsers do not split Set-Cookie on commas, so lx_player_user_token is never stored and the user-token branch of checkPlayerAuth never matches. Pass an array of cookie strings instead.
src/server/server.ts#L1863-L1872: assignloginHeaders['Set-Cookie'] = cookiesand widen the header type toRecord<string, string | string[]>.src/server/server.ts#L4374-L4382: assignregHeaders['Set-Cookie'] = cookiesand widen the header type the same way.
📍 Affects 1 file
src/server/server.ts#L1863-L1872(this comment)src/server/server.ts#L4374-L4382
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/server.ts` around lines 1863 - 1872, Update the login handler
around the loginHeaders construction at src/server/server.ts:1863-1872 to widen
its header type to Record<string, string | string[]> and assign the cookie array
directly to Set-Cookie instead of joining it. Apply the same change to the
registration handler around src/server/server.ts:4374-4382 for regHeaders,
preserving both individual cookie values.
| private signInternalStream(serverId: string, filePath: string): string { | ||
| const secret = global.lx.config['frontend.password'] || 'lxserver-internal' | ||
| const expiry = Date.now() + 600 * 1000 | ||
| const payload = `${serverId}|${filePath}|${expiry}` | ||
| const sig = crypto.createHmac('sha256', secret).update(payload).digest('base64url') | ||
| return `${sig}.${expiry}` | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Do not fall back to a hard-coded HMAC secret.
Line 561 falls back to the literal 'lxserver-internal' when frontend.password is unset or empty. That constant is in the public source. Any deployment that has not set frontend.password uses a key that every reader of this repository knows.
The sst token is the only credential that authorizes /api/openlist/stream and /api/webdav-mounts/stream for a caller with no session cookie. With a known key, an unauthenticated attacker forges a valid sst for any serverId and any filePath. The server then fetches that path from the WebDAV or OpenList mount using its own stored credentials and streams the bytes back. This bypasses both the Subsonic u/p check and the session check.
Two further weaknesses in the same function:
- The key is a user-facing password. It has low entropy and it rotates whenever an administrator changes the password, which silently invalidates issued tokens.
- Line 563 joins fields with an unescaped
|. AfilePaththat contains|makes the payload ambiguous, so distinct(serverId, filePath)pairs can produce the same signed string.
Generate a dedicated random secret at startup, persist it with the other server state, and fail closed if it is missing.
🛡️ Proposed fix
private signInternalStream(serverId: string, filePath: string): string {
- const secret = global.lx.config['frontend.password'] || 'lxserver-internal'
+ const secret = getInternalStreamSecret() // 启动时生成的随机密钥，缺失则抛错
const expiry = Date.now() + 600 * 1000
- const payload = `${serverId}|${filePath}|${expiry}`
+ // 使用 JSON 规范化，避免 filePath 含分隔符时载荷歧义
+ const payload = JSON.stringify([serverId, filePath, expiry])
const sig = crypto.createHmac('sha256', secret).update(payload).digest('base64url')
return `${sig}.${expiry}`
}Add the secret helper next to the other server state, for example:
// src/server/internalStreamToken.ts
import * as crypto from 'crypto'
let secret = ''
export const initInternalStreamSecret = (persisted?: string): string => {
secret = persisted || crypto.randomBytes(32).toString('hex')
return secret
}
export const getInternalStreamSecret = (): string => {
if (!secret) throw new Error('internal stream secret not initialized')
return secret
}Update the verifier in src/server/server.ts to use the same helper and the same canonical payload, and compare with crypto.timingSafeEqual.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/subsonic.ts` around lines 560 - 566, Update signInternalStream to
use a dedicated startup-initialized internal stream secret instead of
global.lx.config['frontend.password'] or any hard-coded fallback. Persist and
restore this secret with server state, fail closed when it is uninitialized, and
canonicalize serverId/filePath/expiry so delimiters such as | cannot create
ambiguous payloads; ensure the corresponding verifier in the server flow uses
the same secret and canonical payload.
| const ws = fs.createWriteStream(tmpPath, { flags: 'w' }) | ||
| resp.on('data', (chunk: any) => { | ||
| received += chunk.length | ||
| ws.write(chunk) | ||
| trackCacheProgress(mount.id, filePath, total, received) | ||
| if (onProgress) onProgress(received, total) | ||
| }) | ||
| resp.on('end', () => { | ||
| ws.end(() => { | ||
| // 上游无 Content-Length（total=0）时，仍要求至少收到数据才落盘； | ||
| // 空响应（received=0）视为无效，清理临时文件避免产生空缓存 | ||
| if (received > 0 && (total === 0 || received >= total)) { | ||
| try { fs.renameSync(tmpPath, cacheFilePath) } catch (e) { | ||
| try { fs.unlinkSync(tmpPath) } catch (e2) { /* ignore */ } | ||
| } | ||
| markCacheDone(mount.id, filePath) | ||
| cleanup(true) | ||
| resolve(true) | ||
| } else { | ||
| cleanup(false) | ||
| resolve(false) | ||
| } | ||
| }) | ||
| }) | ||
| resp.on('error', () => { | ||
| ws.destroy() | ||
| cleanup(false) | ||
| resolve(false) | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Handle write-stream errors and apply backpressure.
Two problems in this block:
- No
errorhandler is attached tows. If the write fails (for exampleENOSPCor a permission error), theWriteStreamemitserrorwith no listener. Node throws an uncaught exception and the process exits.resp.on('error')at Line 521 does not cover write failures. - Line 500 calls
ws.write(chunk)and ignores the return value. There is noresp.pause()/drainhandling. If the WebDAV source is faster than the disk, chunks accumulate in the write buffer. A large FLAC file can grow the heap without bound.
Using resp.pipe(ws) fixes the backpressure problem, and an explicit ws.on('error') fixes the crash.
🐛 Proposed fix
total = parseInt(resp.headers['content-length'] || '0', 10)
trackCacheProgress(mount.id, filePath, total, 0)
const ws = fs.createWriteStream(tmpPath, { flags: 'w' })
+ let failed = false
+ ws.on('error', (e) => {
+ failed = true
+ console.error('[WebDAVMount] cache write failed:', e)
+ resp.destroy()
+ cleanup(false)
+ resolve(false)
+ })
resp.on('data', (chunk: any) => {
received += chunk.length
- ws.write(chunk)
trackCacheProgress(mount.id, filePath, total, received)
if (onProgress) onProgress(received, total)
})
+ resp.pipe(ws)
resp.on('end', () => {
- ws.end(() => {
+ if (failed) return
+ ws.once('finish', () => {
// 上游无 Content-Length（total=0）时，仍要求至少收到数据才落盘；
// 空响应（received=0）视为无效，清理临时文件避免产生空缓存
if (received > 0 && (total === 0 || received >= total)) {resp.pipe(ws) ends ws automatically when resp ends, so wait for finish instead of calling ws.end().
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const ws = fs.createWriteStream(tmpPath, { flags: 'w' }) | |
| resp.on('data', (chunk: any) => { | |
| received += chunk.length | |
| ws.write(chunk) | |
| trackCacheProgress(mount.id, filePath, total, received) | |
| if (onProgress) onProgress(received, total) | |
| }) | |
| resp.on('end', () => { | |
| ws.end(() => { | |
| // 上游无 Content-Length（total=0）时，仍要求至少收到数据才落盘； | |
| // 空响应（received=0）视为无效，清理临时文件避免产生空缓存 | |
| if (received > 0 && (total === 0 || received >= total)) { | |
| try { fs.renameSync(tmpPath, cacheFilePath) } catch (e) { | |
| try { fs.unlinkSync(tmpPath) } catch (e2) { /* ignore */ } | |
| } | |
| markCacheDone(mount.id, filePath) | |
| cleanup(true) | |
| resolve(true) | |
| } else { | |
| cleanup(false) | |
| resolve(false) | |
| } | |
| }) | |
| }) | |
| resp.on('error', () => { | |
| ws.destroy() | |
| cleanup(false) | |
| resolve(false) | |
| }) | |
| const ws = fs.createWriteStream(tmpPath, { flags: 'w' }) | |
| let failed = false | |
| ws.on('error', (e) => { | |
| failed = true | |
| console.error('[WebDAVMount] cache write failed:', e) | |
| resp.destroy() | |
| cleanup(false) | |
| resolve(false) | |
| }) | |
| resp.on('data', (chunk: any) => { | |
| received += chunk.length | |
| trackCacheProgress(mount.id, filePath, total, received) | |
| if (onProgress) onProgress(received, total) | |
| }) | |
| resp.pipe(ws) | |
| resp.on('end', () => { | |
| if (failed) return | |
| ws.once('finish', () => { | |
| // 上游无 Content-Length（total=0）时，仍要求至少收到数据才落盘； | |
| // 空响应（received=0）视为无效，清理临时文件避免产生空缓存 | |
| if (received > 0 && (total === 0 || received >= total)) { | |
| try { fs.renameSync(tmpPath, cacheFilePath) } catch (e) { | |
| try { fs.unlinkSync(tmpPath) } catch (e2) { /* ignore */ } | |
| } | |
| markCacheDone(mount.id, filePath) | |
| cleanup(true) | |
| resolve(true) | |
| } else { | |
| cleanup(false) | |
| resolve(false) | |
| } | |
| }) | |
| }) | |
| resp.on('error', () => { | |
| ws.destroy() | |
| cleanup(false) | |
| resolve(false) | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/webdavMount.ts` around lines 497 - 525, Replace the manual data
handling in the response download flow with resp.pipe(ws) to apply backpressure,
and handle completion via the write stream’s finish event instead of
resp.on('end')/ws.end(). Add an explicit ws.on('error') handler that destroys or
cleans up the response and temporary file, then resolves the operation as
failed; retain the existing successful rename and cache-finalization logic in
the finish path.
- resolveLocalStreamUrl 匹配索引时做路径归一化（URL 解码、合并连续斜杠）， 解决收藏时固化的双斜杠路径（如 /音乐//收藏/）与实时索引单斜杠路径不匹配的问题 - 新增 resolveFromUserList 兜底：实时索引匹配失败时，直接复用歌单中固化的 内部流 url（Web 端已验证可播），并追加 sst 令牌供 Subsonic 客户端无 cookie 播放
箭头音乐等 Subsonic 客户端播放前会用 HEAD /stream 探测资源， 302 后跟随的 HEAD 请求此前因路由仅匹配 GET 而返回 404。 现在 /api/openlist/stream 与 /api/webdav-mounts/stream 同时接受 HEAD， 返回与 GET 一致的响应头（Content-Type/Content-Length）。
- handleGetLyricsBySongId 对 webdav/openlist 源走同目录 .lrc 歌词读取 - 抽取公共 buildLyricResponse，复用 mergedLrc 双行与 structuredLyrics 输出 - resolveMountedLyric 支持索引匹配 + 歌单固化 url 兜底多候选路径 - 歌词文本编码探测：UTF-8 严格解码失败回退 GB18030（远端 GBK .lrc） - openlist 改用原生 httpGetBuffer 获取原始字节，避免 needle 解码损坏
- TextDecoder('gb18030') 在 Alpine node（small-icu）不受支持，GBK .lrc 解码回退 utf-8 导致乱码
- 改为 iconv-lite 的 gb18030 解码（纯 JS 实现，无 ICU 依赖），openlist 与 webdav 两处统一
- package.json / config.js / README 徽章 / docker-compose 镜像 tag 同步至 v3.0.1 - changelog 新增 v3.0.1 条目（挂载、播放修复、歌词、卡密注册等） - 主页登录页与仪表盘展示 v3.0.1 新增功能公告
- requirements.md: Web 播放端仅管理员、有效期管理(7/30/365/永久)、30天活跃≥5分钟自动续期、N天无活跃自动封禁、TG 卡密注册/绑定/改密/线路/歌单自动下载 - design.md: 新增 userAccount.ts / telegramBot.ts(grammY) / playlistParser.ts，扩展 users.json 账号字段，Subsonic 与会话接入到期封禁校验
2 participants
主要改动
/api/openlist/stream、/api/webdav-mounts/stream后返回 401 无法播放挂载音乐。内部流 URL 附加 HMAC 签名令牌sst（含 server+path+过期时间），两个 stream 端点校验放行（timingSafeEqual 防伪造）。nocache=1。Summary by CodeRabbit
New Features
Documentation
Bug Fixes