Add panel backup & restore#9
Conversation
One JSON file with every config table (admins, nodes, accounts+keys, peers, devices, api_keys, panel_settings, audit_log) plus the node-mTLS CA keypair - everything needed to rebuild the panel on a fresh server except deploy/.env. Metrics hypertables are excluded; usage totals live on accounts rows and survive. - store: DumpTables via json_agg in a repeatable-read snapshot; RestoreTables via json_populate_recordset in one atomic tx (truncate all, reinsert in FK order, resync audit_log's sequence). Schema-generic - no per-table structs to drift. Round-trip proven by a new WGPANEL_TEST_POSTGRES_DSN-gated integration test. - httpapi: GET /api/v1/backup and POST /api/v1/backup/restore, both super_admin + audit-logged. Restore validates before touching anything: format, exact applied-migrations match (409 schema_mismatch), and a key canary encrypted with ACCOUNT_KEY_ENCRYPTION_KEY (409 encryption_key_mismatch) since account keys are useless under a different key. Restored CA is written to /data/ca; restart_required flags when it differs from the running one. - frontend: Backup & restore card in Settings with a destructive-confirm restore flow, post-restore summary, and cache clear + re-login advice. - openapi: Backup tag, both endpoints, BackupFile/RestoreResult schemas (public copy synced). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request implements a comprehensive backup and restore feature for the panel, allowing super-admins to download and restore configuration tables and the node-mTLS CA keypair as a single JSON file. The review feedback highlights several critical robustness and reliability improvements: validating that restored tables are not empty to prevent admin lockout, writing CA files atomically to avoid corruption, adding defensive checks against nil pointer panics on s.CA, ensuring cross-browser compatibility for file downloads, and managing post-restore client state transitions to prevent premature redirects.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| counts[table] = int(tag.RowsAffected()) | ||
| } |
There was a problem hiding this comment.
We should validate that the restored admins and panel_settings tables are not empty. If a backup file is missing these tables or they are empty, the restore process will succeed but leave the panel completely locked out (no admin users) or broken (no settings row). Adding a pre-commit validation prevents this critical failure mode.
counts[table] = int(tag.RowsAffected())
}
if counts["admins"] == 0 {
return nil, fmt.Errorf("restore aborted: backup must contain at least one admin user to prevent lockout")
}
if counts["panel_settings"] == 0 {
return nil, fmt.Errorf("restore aborted: backup must contain at least one panel_settings row")
}| func (s *Server) writeRestoredCA(ca backupCA) error { | ||
| if err := os.MkdirAll(s.CADataDir, 0o700); err != nil { | ||
| return err | ||
| } | ||
| if err := os.WriteFile(filepath.Join(s.CADataDir, "ca-cert.pem"), []byte(ca.CertPEM), 0o600); err != nil { | ||
| return err | ||
| } | ||
| return os.WriteFile(filepath.Join(s.CADataDir, "ca-key.pem"), []byte(ca.KeyPEM), 0o600) | ||
| } |
There was a problem hiding this comment.
Writing the CA certificate and key files directly using os.WriteFile is not atomic. If the write fails halfway or the system crashes, the files can be left in an inconsistent or corrupted state. It is much safer to write them to temporary files in the same directory and then atomically rename them.
func (s *Server) writeRestoredCA(ca backupCA) error {
if err := os.MkdirAll(s.CADataDir, 0o700); err != nil {
return err
}
certTmp, err := os.CreateTemp(s.CADataDir, "ca-cert.*.pem")
if err != nil {
return err
}
defer os.Remove(certTmp.Name())
if _, err := certTmp.Write([]byte(ca.CertPEM)); err != nil {
certTmp.Close()
return err
}
if err := certTmp.Chmod(0o600); err != nil {
certTmp.Close()
return err
}
if err := certTmp.Close(); err != nil {
return err
}
keyTmp, err := os.CreateTemp(s.CADataDir, "ca-key.*.pem")
if err != nil {
return err
}
defer os.Remove(keyTmp.Name())
if _, err := keyTmp.Write([]byte(ca.KeyPEM)); err != nil {
keyTmp.Close()
return err
}
if err := keyTmp.Chmod(0o600); err != nil {
keyTmp.Close()
return err
}
if err := keyTmp.Close(); err != nil {
return err
}
if err := os.Rename(certTmp.Name(), filepath.Join(s.CADataDir, "ca-cert.pem")); err != nil {
return err
}
return os.Rename(keyTmp.Name(), filepath.Join(s.CADataDir, "ca-key.pem"))
}| resp.CARestored = true | ||
| resp.RestartRequired = !bytes.Equal(bytes.TrimSpace(s.CA.CertPEM), bytes.TrimSpace([]byte(backup.CA.CertPEM))) |
There was a problem hiding this comment.
If s.CA is nil (for example, in certain test environments or if the CA is disabled/unconfigured), dereferencing s.CA.CertPEM will cause a nil pointer panic. It is safer to add a defensive check for s.CA != nil before accessing its fields.
resp.CARestored = true
if s.CA != nil {
resp.RestartRequired = !bytes.Equal(bytes.TrimSpace(s.CA.CertPEM), bytes.TrimSpace([]byte(backup.CA.CertPEM)))
} else {
resp.RestartRequired = true
}| const a = document.createElement('a') | ||
| a.href = url | ||
| a.download = filename | ||
| a.click() | ||
| URL.revokeObjectURL(url) |
There was a problem hiding this comment.
In some browsers (especially older Firefox or certain mobile browsers), calling a.click() on an anchor element that is not attached to the DOM might be ignored or blocked. It is safer to temporarily append the element to document.body before clicking it, and then remove it.
| const a = document.createElement('a') | |
| a.href = url | |
| a.download = filename | |
| a.click() | |
| URL.revokeObjectURL(url) | |
| const a = document.createElement('a') | |
| a.href = url | |
| a.download = filename | |
| document.body.appendChild(a) | |
| a.click() | |
| document.body.removeChild(a) | |
| URL.revokeObjectURL(url) |
| // Everything cached client-side describes the pre-restore panel. | ||
| queryClient.clear() | ||
| push('success', 'Backup restored') |
There was a problem hiding this comment.
If the application has a global HTTP 401 interceptor that automatically redirects the user to the login page on any unauthorized request, calling queryClient.clear() might trigger immediate background refetches using the now-invalid token. This would cause the user to be instantly redirected to the login page, preventing them from reading the restore success message or the important CA restart warning. Consider handling the post-restore state transition carefully (e.g., showing a blocking modal or delaying the logout/redirect).
What
Settings → Backup & restore (super_admin only):
Safety rails
Restore validates before touching anything:
wgpanel-backup/1409 schema_mismatch(json_populate_recordset against a drifted schema fails in data-dependent ways)ACCOUNT_KEY_ENCRYPTION_KEY→409 encryption_key_mismatch(account keys in the file are useless under a different key)Restore is one transaction (truncate + reinsert in FK order, audit_log sequence resynced); a
backup.restoredaudit entry with counts lands in the restored timeline. If the restored CA differs from the running one, the response flagsrestart_required(agents keep the old CA untilwgpanel restart).Verification
TestBackupRoundTripintegration test (gated onWGPANEL_TEST_POSTGRES_DSN, same asTestStoreIntegration) run against a real TimescaleDB container: dump → mutate → restore is lossless, sequence resync works, unknown table names rejected.gofmt/go vet/go test ./...clean; frontendtsc+ build clean.🤖 Generated with Claude Code