From 3a1746b0c00f620f6840e23020c609fe3bb6db7e Mon Sep 17 00:00:00 2001 From: Myron Blair Date: Tue, 23 Jun 2026 03:13:41 +0000 Subject: [PATCH] fix: all 6 code review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. admin.js: dashboard setTimeout was after return (dead code) — restructured to assign template to const html, run setTimeout, then return html 2. DockerManager.php createStack: replaced SELECT LAST_INSERT_ID() with db->insert() which already returns lastInsertId correctly for SQLite 3. DockerManager.php setQuota: replaced ON DUPLICATE KEY UPDATE / VALUES() MySQL syntax with SQLite-compatible ON CONFLICT(user_id) DO UPDATE SET excluded.col syntax 4. post-restore.sh: PHP helper file now written ONCE at start of step 4 before any call to it (was written AFTER first call, causing silent failure) 5. post-restore.sh: git pull exit code now captured before pipeline (the while-read loop always exited 0, masking pull failures) 6. uninstall.sh: tar backup now aborts on failure (previously 2>/dev/null swallowed errors and rm -rf destroyed source unconditionally); also rm -f → rm -rf for .service.d drop-in directory --- deploy/novacpx-post-restore.sh | 40 +++++++++++++++++---------------- panel/lib/DockerManager.php | 8 ++++--- panel/public/assets/js/admin.js | 4 +++- uninstall.sh | 11 ++++++--- 4 files changed, 37 insertions(+), 26 deletions(-) diff --git a/deploy/novacpx-post-restore.sh b/deploy/novacpx-post-restore.sh index df97c61..f9860bc 100755 --- a/deploy/novacpx-post-restore.sh +++ b/deploy/novacpx-post-restore.sh @@ -51,7 +51,10 @@ if [[ "$SKIP_GIT" != "--no-git" ]] && [[ -n "$PAT" ]]; then cd /opt/novacpx-src 2>/dev/null || { warn "novacpx-src not found, skipping"; true; } if [[ -d /opt/novacpx-src/.git ]]; then git remote set-url origin "$NOVACPX_REPO" 2>/dev/null - git pull origin main 2>&1 | tail -3 | while read l; do log " $l"; done + # Capture exit code separately — pipeline masks it via while loop + GIT_OUT=$(git pull origin main 2>&1) && GIT_OK=1 || GIT_OK=0 + echo "$GIT_OUT" | tail -3 | while read l; do log " $l"; done + [[ "$GIT_OK" == "0" ]] && { warn "git pull failed — deploying existing local code"; } rsync -a --delete --exclude=".git" --exclude="api/config.php" \ /opt/novacpx-src/panel/public/ /srv/novacpx/public/ 2>/dev/null rsync -a --delete --exclude="config.php" \ @@ -79,24 +82,8 @@ fi log "4. Fixing webacct hosting account..." DB=/var/lib/novacpx/panel.db -# Clean orphaned user record (user exists in DB but Linux user doesn't) -if id "webacct" &>/dev/null; then - ok "webacct Linux user exists" -else - warn "webacct Linux user missing — cleaning DB and recreating..." - # Remove orphaned DB record - sqlite3 "$DB" "DELETE FROM users WHERE username='webacct' AND id NOT IN (SELECT user_id FROM accounts WHERE username='webacct');" 2>/dev/null || true - sqlite3 "$DB" "DELETE FROM users WHERE username='webacct' AND role='user';" 2>/dev/null || true - - # Create via PHP - php8.3 /tmp/_nova_create_webacct.php >> "$LOG" 2>&1 || true -fi - -# Ensure account exists in DB -ACCT_EXISTS=$(sqlite3 "$DB" "SELECT COUNT(*) FROM accounts WHERE username='webacct';" 2>/dev/null || echo "0") -if [[ "$ACCT_EXISTS" == "0" ]]; then - warn "webacct account not in DB — creating..." - cat > /tmp/_nova_create_webacct.php << 'PHPEOF' +# Write the PHP helper ONCE here, before any call to it +cat > /tmp/_nova_create_webacct.php << 'PHPEOF' insert( $r = AccountManager::create(['username'=>'webacct','domain'=>'web.orbishosting.com','password'=>'Joker1974!!!','user_id'=>$uid,'php_version'=>'8.3']); echo "Created: ".json_encode($r)."\n"; PHPEOF + +# Clean orphaned user record (Linux user missing but DB record exists) +if id "webacct" &>/dev/null; then + ok "webacct Linux user exists" +else + warn "webacct Linux user missing — cleaning DB and recreating..." + sqlite3 "$DB" "DELETE FROM users WHERE username='webacct' AND id NOT IN (SELECT user_id FROM accounts WHERE username='webacct');" 2>/dev/null || true + sqlite3 "$DB" "DELETE FROM users WHERE username='webacct' AND role='user';" 2>/dev/null || true + php8.3 /tmp/_nova_create_webacct.php >> "$LOG" 2>&1 || true +fi + +# Ensure account exists in DB +ACCT_EXISTS=$(sqlite3 "$DB" "SELECT COUNT(*) FROM accounts WHERE username='webacct';" 2>/dev/null || echo "0") +if [[ "$ACCT_EXISTS" == "0" ]]; then + warn "webacct account not in DB — creating..." php8.3 /tmp/_nova_create_webacct.php >> "$LOG" 2>&1 && ok "Account created" || warn "Account creation failed — check log" fi diff --git a/panel/lib/DockerManager.php b/panel/lib/DockerManager.php index 1c45503..3148254 100644 --- a/panel/lib/DockerManager.php +++ b/panel/lib/DockerManager.php @@ -252,11 +252,10 @@ SH; } if (!is_dir($dir)) throw new RuntimeException("Failed to create stack directory: {$dir}"); file_put_contents("{$dir}/docker-compose.yml", $composeYaml); - $this->db->execute( + $id = (int)$this->db->insert( "INSERT INTO docker_compose_stacks (account_id, name, stack_dir, compose_file, status) VALUES (?,?,?,?,'pending')", [$accountId, $safeName, $dir, $composeYaml] ); - $id = $this->db->fetchOne("SELECT LAST_INSERT_ID() as id")['id']; novacpx_log('info', "DockerManager: created stack {$safeName}"); return ['id' => $id, 'dir' => $dir]; } @@ -291,7 +290,10 @@ SH; $this->db->execute( "INSERT INTO docker_quotas (user_id, max_containers, max_memory_mb, max_cpus) VALUES (?,?,?,?) - ON DUPLICATE KEY UPDATE max_containers=VALUES(max_containers), max_memory_mb=VALUES(max_memory_mb), max_cpus=VALUES(max_cpus)", + ON CONFLICT(user_id) DO UPDATE SET + max_containers=excluded.max_containers, + max_memory_mb=excluded.max_memory_mb, + max_cpus=excluded.max_cpus", [$userId, $maxContainers, $maxMemoryMb, $maxCpus] ); } diff --git a/panel/public/assets/js/admin.js b/panel/public/assets/js/admin.js index f0e5488..312380e 100644 --- a/panel/public/assets/js/admin.js +++ b/panel/public/assets/js/admin.js @@ -124,7 +124,7 @@ document.getElementById('server-ip').textContent = ''; - return ` + const html = `
CPU Usage
@@ -198,7 +198,9 @@ : ''}
`; + // setTimeout BEFORE return so it actually executes (after return is dead code) setTimeout(()=>{const c=document.getElementById('dash-hist-chart');if(!c||!hist.length)return;if(window.Chart){initStatsChart(c,hist);}else{const s2=document.createElement('script');s2.src='https://cdn.jsdelivr.net/npm/chart.js@4.4.0/dist/chart.umd.min.js';s2.onload=()=>initStatsChart(c,hist);document.head.appendChild(s2);}},150); + return html; } // ── Server Status ────────────────────────────────────────────────────────── diff --git a/uninstall.sh b/uninstall.sh index c42ac54..a1d8e55 100755 --- a/uninstall.sh +++ b/uninstall.sh @@ -70,8 +70,12 @@ log "Cron jobs" [[ -d /etc/postfix ]] && cp -a /etc/postfix "$BACKUP_DIR/configs/postfix" 2>/dev/null || true [[ -d /etc/dovecot ]] && cp -a /etc/dovecot "$BACKUP_DIR/configs/dovecot" 2>/dev/null || true -# Compress backup -tar -czf "$BACKUP_ARCHIVE" -C "$(dirname $BACKUP_DIR)" "$(basename $BACKUP_DIR)" 2>/dev/null +# Compress backup — abort if tar fails rather than silently delete unbackedup files +tar -czf "$BACKUP_ARCHIVE" -C "$(dirname $BACKUP_DIR)" "$(basename $BACKUP_DIR)" || { + echo -e "${RED}[✗]${NC} Backup archive creation FAILED. Uninstall aborted to protect your data." + echo " Staging dir preserved at: $BACKUP_DIR" + exit 1 +} rm -rf "$BACKUP_DIR" BACKUP_SIZE=$(du -sh "$BACKUP_ARCHIVE" | cut -f1) log "Backup archive: $BACKUP_ARCHIVE ($BACKUP_SIZE)" @@ -131,7 +135,8 @@ systemctl reload php8.3-fpm 2>/dev/null || true step "Removing systemd units" for svc in novacpx-web; do systemctl stop "$svc" 2>/dev/null; systemctl disable "$svc" 2>/dev/null - rm -f "/etc/systemd/system/${svc}.service" "/etc/systemd/system/${svc}.service.d" + rm -f "/etc/systemd/system/${svc}.service" + rm -rf "/etc/systemd/system/${svc}.service.d" log "Removed: $svc" done systemctl daemon-reload