From f65af5d7fd4cadc31631bbb31408a19f4c1df48b Mon Sep 17 00:00:00 2001 From: Sami Date: Tue, 12 May 2026 18:20:59 -0700 Subject: [PATCH] fix(updater): stop false-positive "update available" loop when commit is unknown Dockerfile never received DASHCADDY_COMMIT at build, so /app/VERSION held 'unknown'. _isNewer then treated same-version-different-commit as newer, making the auto-updater rebuild the container indefinitely (each rebuild still produced commit='unknown'). - self-updater._isNewer: normalize commits; treat unknown/null/empty as no commit info and fall back to pure version comparison - self-updater._autoCheckAndApply + routes/updates: refuse to apply when local version >= remote version (belt-and-suspenders) - update-management.js: hide '(unknown)' from version label - Dockerfile: COPY VERSION instead of writing from build arg - VERSION: committed placeholder ('dev'); scripts/release.sh now writes the real short SHA into the tarball's VERSION before tar-ing, so every published release ships with an accurate commit Co-Authored-By: Claude Opus 4.7 (1M context) --- dashcaddy-api/Dockerfile | 6 ++++-- dashcaddy-api/VERSION | 1 + dashcaddy-api/routes/updates.js | 13 +++++++++++-- dashcaddy-api/self-updater.js | 30 +++++++++++++++++++++++++----- scripts/release.sh | 3 +++ status/js/update-management.js | 3 ++- 6 files changed, 46 insertions(+), 10 deletions(-) create mode 100644 dashcaddy-api/VERSION diff --git a/dashcaddy-api/Dockerfile b/dashcaddy-api/Dockerfile index 2680695..6d33bc0 100644 --- a/dashcaddy-api/Dockerfile +++ b/dashcaddy-api/Dockerfile @@ -13,8 +13,10 @@ COPY src/ ./src/ COPY routes/ ./routes/ COPY openapi.yaml ./ -ARG DASHCADDY_COMMIT=unknown -RUN echo "${DASHCADDY_COMMIT}" > VERSION +# VERSION file holds the short git SHA the image was built from. Committed as +# 'dev' for source builds; the release script (scripts/release.sh) overwrites it +# with the actual commit hash before tarballing each release. +COPY VERSION ./ # Note: Running as root because container needs Docker socket access # (which is root-equivalent anyway). Socket access required for container management. diff --git a/dashcaddy-api/VERSION b/dashcaddy-api/VERSION new file mode 100644 index 0000000..38f8e88 --- /dev/null +++ b/dashcaddy-api/VERSION @@ -0,0 +1 @@ +dev diff --git a/dashcaddy-api/routes/updates.js b/dashcaddy-api/routes/updates.js index 62f3507..4211f8e 100644 --- a/dashcaddy-api/routes/updates.js +++ b/dashcaddy-api/routes/updates.js @@ -95,6 +95,15 @@ module.exports = function({ updateManager, selfUpdater, asyncHandler, logError } if (!check.available) { return res.json({ success: true, message: 'Already up to date' }); } + // Refuse same-version applies. The check.available flag can theoretically be + // true with equal versions (commit-mismatch path); applying anyway just + // rebuilds the container without changing anything user-visible and pollutes + // history with v1.4.0 → v1.4.0 entries. + const localV = check.local && check.local.version; + const remoteV = check.remote && check.remote.version; + if (localV && remoteV && localV === remoteV) { + return res.json({ success: true, message: 'Already up to date', version: localV }); + } // Start async — container may restart selfUpdater.applyUpdate(check.remote).catch(err => { logError('self-update', err); @@ -102,8 +111,8 @@ module.exports = function({ updateManager, selfUpdater, asyncHandler, logError } res.json({ success: true, message: 'Update initiated', - fromVersion: check.local.version, - toVersion: check.remote.version, + fromVersion: localV, + toVersion: remoteV, }); }, 'system-update-apply')); diff --git a/dashcaddy-api/self-updater.js b/dashcaddy-api/self-updater.js index 665f5ea..2de8eaf 100644 --- a/dashcaddy-api/self-updater.js +++ b/dashcaddy-api/self-updater.js @@ -400,10 +400,17 @@ class SelfUpdater extends EventEmitter { async _autoCheckAndApply() { try { const result = await this.checkForUpdate(); - if (result.available && result.remote) { - console.log('[SelfUpdater] Update available: %s → %s', result.local.version, result.remote.version); - await this.applyUpdate(result.remote); + if (!result.available || !result.remote) return; + // Belt-and-suspenders: never auto-apply a same-version update. A bug here + // creates an infinite rebuild loop (each fresh container has commit='unknown' + // so it keeps comparing as different from remote forever). + if (this._compareVersions(result.local.version, result.remote.version) >= 0) { + console.log('[SelfUpdater] Skipping auto-apply: local %s is not older than remote %s', + result.local.version, result.remote.version); + return; } + console.log('[SelfUpdater] Update available: %s → %s', result.local.version, result.remote.version); + await this.applyUpdate(result.remote); } catch (e) { console.error('[SelfUpdater] Auto-update error:', e.message); } @@ -496,11 +503,24 @@ class SelfUpdater extends EventEmitter { const versionCompare = this._compareVersions(local.version || '0.0.0', remote.version); if (versionCompare < 0) return true; if (versionCompare > 0) return false; - // Same version — check commit hash - if (remote.commit && local.commit && remote.commit !== local.commit) return true; + // Same version — only flag as newer if BOTH commits are known and differ. + // If local.commit is missing or 'unknown' (Dockerfile default when no build arg + // is passed), we can't distinguish builds, so trust the version number and + // treat them as equivalent. Otherwise the updater loops forever applying + // the same version because every fresh container build has commit='unknown'. + const localCommit = this._normalizeCommit(local.commit); + const remoteCommit = this._normalizeCommit(remote.commit); + if (localCommit && remoteCommit && localCommit !== remoteCommit) return true; return false; } + _normalizeCommit(value) { + if (!value) return null; + const str = String(value).trim().toLowerCase(); + if (!str || str === 'unknown' || str === 'null' || str === 'undefined') return null; + return str; + } + _compareVersions(a, b) { const av = String(a || '0.0.0').split('.').map(part => parseInt(part, 10) || 0); const bv = String(b || '0.0.0').split('.').map(part => parseInt(part, 10) || 0); diff --git a/scripts/release.sh b/scripts/release.sh index 0eed420..0ca2163 100644 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -84,6 +84,9 @@ ssh "$RELEASE_HOST" "set -e fi rm -rf .git find . -type d -name node_modules -exec rm -rf {} + 2>/dev/null || true + # Bake the actual commit SHA into the api VERSION file so containers built + # from this tarball report a real commit (not the 'dev' placeholder). + echo \"$COMMIT\" > dashcaddy-api/VERSION cd /tmp/dashcaddy-build tar zcf dashcaddy-$VERSION.tar.gz dashcaddy/ " diff --git a/status/js/update-management.js b/status/js/update-management.js index 2ae6e98..03d5f5c 100644 --- a/status/js/update-management.js +++ b/status/js/update-management.js @@ -335,7 +335,8 @@ const res = await fetch('/api/v1/system/version'); const data = await res.json(); if (data.success) { - dcVersionInfo.textContent = 'v' + data.version + (data.commit ? ' (' + data.commit.substring(0, 7) + ')' : ''); + const commit = data.commit && data.commit !== 'unknown' ? data.commit : null; + dcVersionInfo.textContent = 'v' + data.version + (commit ? ' (' + commit.substring(0, 7) + ')' : ''); } } catch (_) { dcVersionInfo.textContent = 'Unable to fetch version';