Copilot review on the GHSA-rm3m-9mvw-68fh patch caught two issues:
- _BACKEND_DIR/_INSTALL_DIR were one level too shallow (backend/app and
backend/ instead of backend/ and the install root), which left
<install>/.env and frontend/dist reachable on the deployed layout —
the exact PoC path from the advisory. _BACKEND_DIR stays in
PROTECTED_ROOTS so flat layouts (code at /app) are covered too.
- The ALLOWED_ROOTS check used a bare startswith, so e.g. /optfoo
matched the /opt root. Both checks now require an exact match or a
path-boundary prefix.
New tests pin the level math (the existing cases computed the expected
layout independently and passed on Windows for the wrong reason) and
simulate the deployed layout with the install dir inside an allowed root.
New builtin job security-feed (daily) pulls the normalized advisory feed
from serverkit.ai/api/security-feed.json (a cached proxy of the repo's
published GitHub Security Advisories) and:
- notifies admins (security.alert, critical) when the running panel
version falls inside an advisory's affected range, once per advisory;
- fires a one-time post-fix reminder after an upgrade crosses a fix
boundary when the advisory declares a "## Post-upgrade actions" section
(e.g. rotate JWT_SECRET_KEY after GHSA-rm3m-9mvw-68fh).
Dedupe keys, the last-good feed copy, and the previously-seen version live
in SystemSettings, so restarts never re-notify and outbound failures serve
the cached feed. Opt out via the security_feed.enabled setting. Privacy
page on serverkit.ai updated to disclose the daily call.
The 1.7.68 RBAC fix enforced files.read on read endpoints, but the default
viewer role legitimately has files.read=true, and on the documented
install.sh layout the panel install dir (/opt/serverkit) sits under the
allowed root /opt. A viewer could read the backend .env, steal
JWT_SECRET_KEY / SERVERKIT_ENCRYPTION_KEY, and forge admin sessions.
FileService now carries PROTECTED_ROOTS (the panel install dir and
SERVERKIT_CONFIG_DIR), checked inside is_path_allowed before the
ALLOWED_ROOTS test, so every file-manager operation (read, write, browse,
download, upload, ...) refuses panel-internal paths for every role.
Regression tests: protected paths rejected by is_path_allowed; viewer
read/download of the backend .env returns 403; allowed roots keep working.
Co-authored-by: CaptBoykin <CaptBoykin@users.noreply.github.com>
- install_profile_service: record what install.sh put on the box
(minimal/standard/full), recommended from detected hardware and
overridable; thresholds mirrored by recommend_profile in install.sh
- setup wizard: replace the hard ResourceGate/SetupStepTier with
SetupStepCapacity + SetupStepSecurity
- ResourceAdvisory: inline capacity warning at the point of action
(WordPress/Docker/Services/Deployments), backed by headroom.fits
- RequiresDocker component for docker-dependent surfaces
- install.sh/update.sh: profile detection + tests
(test_install.sh, test_update.sh, test_server_capacity.py)
components/PageSkeleton.jsx already owned `.sk-page` for its loading
archetypes, including `gap: $space-4`. The new shell reused the name, so every
page built on PageLayout inherited a 16px flex gap between the top bar and the
content — invisible on padded pages, but on Terminal/Logs it showed as a band
of page background under the bar, which is exactly the "weird space" reported.
The shell is `.sk-pageshell` now; the skeleton keeps `.sk-page`. The smoke test
gained a vgap assertion (content top must equal the bar's bottom edge), which
is what would have caught this in the first place.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three detail panels rendered their own `.preview-drawer` — fixed positioning,
their own backdrop, their own slide animation, their own close button — so they
had no focus trap, no escape-to-close, and header chrome that only resembled
the shared drawer's. The Terminal process and service panels and Docker's
container logs panel now use ds/Drawer like everything else.
Left alone deliberately: the File Manager preview (it also renders inline as a
column pane, which a Radix sheet cannot do) and Docker's container inspector
(the `dx-*` workspace has its own language by design).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every drawer body had to remember to pad itself, and they had drifted into
three answers: 18px (filters, telemetry, incidents, monitors), 20/20/32
(domains, cron), and nothing at all — backup detail, restore, cutover and the
scheduled-tasks runs list were rendering flush against the panel edge, the same
defect just reported on the cron create drawer. Ten drawers, three insets.
The body is now padded to the header's own 18px inset, and the seven bodies
that genuinely own their internal regions — a form with a pinned footer
(Templates, WordPress), a two-pane catalog (both database engine drawers), an
editor (widget editor), a filtered library (widget library), a mode switcher
(Servers) — declare `flush` instead of relying on the absence of a default.
The now-duplicate per-drawer padding is deleted.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Git hand-rolled the tab-group frame — `page-container--full-bleed sk-tabgroup`
plus its own `__content`/`__inner` wrappers, duplicated across the loading
branch and the main return. It renders through the shared layout now, so the
frame comes from one place and the loading state matches the loaded page.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The console panes were rounded, bordered cards floating inside a `0 16px 16px`
inset — the only page framed that way, which read as stray side padding plus
stray corners against an otherwise flush layout. It now renders through
PageLayout in `fill` mode, like the File Manager: the panes fill the content
region edge-to-edge and keep their internal dividers. Services stays a card
grid, so it re-pads itself instead of running to the edges.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`.sk-drawer__body` is the scroll region and deliberately carries no padding —
some drawers run edge-to-edge — so each body pads its own root. `.cron-form`
did not, leaving every label and input flush against the panel edges. Same
inset as `.cron-drawer` so the two cron drawers line up, plus a note on the DS
Drawer so the next body does not repeat it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Creating a job went through a plain modal with a preset <select> and a bare
cron text field — the schedule was something you had to already know how to
write. It now opens a drawer built on the shared SchedulePicker (presets ·
builder · advanced, with the humanized cadence and the next fire times), plus a
preview of the exact crontab line the form will write.
The drawer owns its own form state, so the page drops the form object, the
reset/open/submit handlers and the /cron/presets fetch that only fed the old
dropdown. The list head gains the section title and the "N of M jobs" count,
with the filter moved to the right where the mock has it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
components/_schedule-picker.scss was never added to main.scss, so the shared
cron picker has been rendering unstyled everywhere it is used — raw bordered
boxes with the preset label and its cron expression jammed together, and the
next-run list running into the description. Adding the one import restores it
for Backups' schedule card and the server-detail cron tab as well.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fourteen pages each carried their own frame: some a padded `.page-container`,
some a bare fragment, some the `app-detail-page` chrome with a per-page
`.main-content:has(...)` escape hatch to undo the frame again. All of them now
render through PageLayout, which deletes the per-page shell code and puts every
top bar flush against the content region.
Loading and not-found early returns take the same shell as the loaded page, so
the bar no longer appears, shifts, or vanishes as data resolves. The now-dead
`.main-content:has(.ws-detail-page)` rule goes with it — the shared full-bleed
rule covers it, and two :has() rules fought each other on small screens.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Standalone pages had no shared shell, so each one hand-rolled its frame by
dropping a PageTopbar into the default padded `.page-container`. The bar
carries the sidebar's surface and a bottom border precisely because it is
chrome meant to pin to the frame — inside a padded, max-width-capped well it
instead floated in the middle of the content area with a gutter on every side.
PageLayout gives those pages the same frame TabGroupLayout already gave a tab
group: full-bleed shell, top bar flush to the top of the content region, body
scrolling beneath it in the reading column. The shell rules now live in three
mixins shared by `.sk-page` and `.sk-tabgroup` so there is one definition of
what a page frame is.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Monitors, a monitor's detail and 90-day uptime strip, Incidents, Events and
Jobs. These double as the render smoke test for the pages this round added.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- The Events severity chip passed a MetricCard tone to Pill. The two token sets
overlap but are not the same — Pill has no "accent", so info events rendered
as bare text with no chip while every other severity had one. Split the config
into `tone` (tile) and `pill` (chip).
- "128 ms" wrapped onto two lines in the Monitors table's narrow response
column.
Both were only visible in a rendered page, not in lint or the build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Walks the AST of monitor_service and the monitors blueprint to assert neither
imports the plugin loader nor reaches for StatusPageService, and that the routes
register without the extension. This is the property the promotion exists to
provide — watching a site must keep working on a lean panel with
serverkit-status uninstalled — so it should fail loudly if it regresses.
AST rather than a source grep: the modules' own comments discuss the extension,
which a text match would trip over.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
_maybe_attach_certificate throttled on last_check_at, which is the probe clock.
An active monitor is checked every 30s, so that condition was never older than
the 6h refresh window: the certificate would be read once and then never
refreshed, leaving a stale expiry date on the detail page forever.
Adds cert_checked_at (migration 081) and throttles on that instead. The stamp
is written even when the read fails, so an unreachable TLS endpoint is retried
on the same cadence rather than on every single probe.
Also serves the Monitors list sparkline from one bulk query per page instead of
one per row, bounded by a time window so a busy monitor's history cannot crowd
out a quiet one's the way a single global LIMIT would.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The group's information architecture had drifted. The Events tab pointed at
/telemetry, which was nested inside the monitoring TabGroupLayout but ALSO
rendered its own PageTopbar — two stacked headers. Jobs sat as a top-level
sidebar entry while being the same class of thing as Events, so the two read as
rival pages.
- MONITOR_TABS is now Overview / Monitors / Incidents / Rules / Capacity /
Events / Jobs / Doctor (+ Status Pages from the extension). Alerts is gone as
a tab; /monitoring/alerts and /monitoring/fleet-alerts redirect to Incidents,
and FleetAlertsPanel moved there with its own ack/resolve lifecycle intact.
- Jobs moves to /monitoring/jobs and loses its sidebar entry; /jobs redirects.
Activity/Scheduled becomes an in-page SegControl since the group's bar already
owns the tab row. JOBS_TABS is deleted.
- Events (Telemetry) rebuilt on the standard pattern: no PageTopbar of its own,
KpiBand + MetricCard severity filters instead of bespoke telemetry-stat divs,
topbar SearchField + FilterDrawer instead of an inline collapsible panel,
DataTable + DS Drawer. Every capability is kept — correlation drill-down, all
filters, test-event emit, cleanup, pagination.
- The Overview leads with a monitors summary (availability, live check feed,
monitor list); host resources keep everything they had, one section down.
- Status pages can now publish a monitor that already exists, and detach it
again without destroying it — deleting a component now deletes a real monitor.
FilterDrawer grows `children` + `onClear` so a host can add non-list filters
(free text, date bounds) and still have "Clear all" reach them; without onClear
it would silently reset only the chip groups.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Monitoring only ever described the machine the panel runs on. These are the
screens for the things it is supposed to be watching:
- /monitoring/monitors — the check list on the Domains/Cron pattern: clickable
KPI tiles as the status filter, topbar SearchField + FilterDrawer, and a table
with a response sparkline, 30-day uptime and a live next-check countdown.
Creating a monitor picks a check type (HTTP / keyword / port / ping / DNS /
SMTP) and its probe options.
- /monitoring/monitors/:id — the drill-down, outside the tab group so it carries
its own breadcrumb header. Sections are a SegControl, not a second tab strip:
Performance (p50 chart + stat strip), Uptime (90-day bars + certificate),
Check log (freezable so a streaming table stays readable), Configuration.
- /monitoring/incidents — absorbs the old Alerts tab. A CPU threshold crossing
and a monitor going down are the same question asked of two subjects, and
splitting them is what made alerting look like it only described this server.
Host threshold alerting is unchanged, just rendered in the same timeline.
- UptimeBars renders a day with no samples as its own "unwatched" state rather
than a full green bar — "we weren't looking" and "it was fine" are different
facts, and conflating them is how uptime widgets lie about their history.
The list sparkline is fed by one bulk query for the whole page, not one per row.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A StatusComponent was already a full synthetic check (http/tcp/dns/smtp/ping,
target, interval, timeout, HealthCheck history, uptime windows, auto-incidents)
— it was just framed as "a component of a status page" and its engine lived
inside the serverkit-status extension, so watching a site required publishing a
status page. Promote it:
- migration 081 makes status_components.page_id and status_incidents.page_id
nullable, and adds the columns backing the monitor UI (is_paused,
check_method, expected_status, keyword, follow_redirects, verify_tls, retries,
consecutive_failures, cert_issuer, cert_expires_at)
- new core app/services/monitor_service.py owns the check engine; the extension
keeps page publishing and delegates
- new /api/v1/monitors (CRUD, check, pause, history, uptime buckets, incidents)
- new builtin.monitor_check sweep on a 30s tick — until now checks only ran when
someone pressed "Check now", so a configured monitor never noticed an outage
Check-engine defects fixed while moving it:
- ping ignored the process return code and could only ever report 'up'
- ping used Linux-only flags with no guard, breaking the Windows dev server
- http ignored its own method/expected_status/follow_redirects/verify_tls
- smtp was documented on the column but unimplemented; keyword did not exist
- run_check never recomputed uptime and never opened or resolved an incident —
only the WordPress health-sync path did, so network monitors never paged
- TLS issuer/expiry are now captured (throttled to one read per 6h)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
With /cron, /webhooks and /test-sandbox off it, the old
<div class="page-header"><h1>...</h1></div> block had one live user
left: Docker's Docker-not-installed branch, where the panel below it
already supplies its own heading. Remove that, and the class is dead.
Delete the base rule and the overrides that outlived it in
_docker.scss, _marketplace.scss, _wordpress.scss,
_wordpress-pipeline.scss and components/_tabs.scss, plus the unused
.docker-page-header. The base rule's slot keeps a comment saying what
replaced it.
StyleGuide's "Page Headers" section still described this as "the
canonical pattern, use this for all pages", which is how it kept
getting picked for new pages. It now demos the two live shapes — the
titleless TabGroupLayout bar (the default) and the titled standalone
bar — and names .page-header as retired. StyleGuide's own header moves
to PageTopbar.
Docker and the Database Explorer intentionally carry no page bar at
all; that is unchanged and deliberate.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The page kept a shape the rest of the app moved off: a four-card KPI
strip, its own in-body search input, a hand-rolled table, four icon
buttons per row, and an edit modal on row click.
It now matches /domains: the segmented filter carries the counts (no
KPI strip), SearchField lives in the top bar, the list is the shared
DataTable, and a row opens the DS Drawer. The top bar keeps its title
— /cron has no tab group, and the reference mock shows a titled bar
there; the body was what was stale.
Last run / Next run / Status columns need data list_jobs() never
returned: it skipped the schedule_human / next_run / run-tracking joins
that jobs_for_application() has done since plan 34, so the admin list
could only ever show enabled-vs-disabled. Apply the same enrichment,
with tests covering the schedule join, the last-run join, and the
no-history case.
The drawer's run history uses GET /cron/jobs/<id>/runs, which already
existed and only ScheduledTasksCard consumed. It's admin-only and
empty until a job carries the tracking shim, so both cases are stated
in the drawer (with an enable-tracking action) rather than rendering a
blank table.
Also fixes styles that never applied: .run-output-*, .spinner-inline
and the modal field overrides were nested under .cron-page, but Radix
Dialog and Sheet portal to <body>. Hoisted to top level.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two sidebar surfaces that shouldn't have been top-level pages. Both
touch App.jsx + sidebarItems.js, so they land together.
Test Sandbox: the distro-matrix console shipped visible and routable
on every production install. Add a shared useDevMode() hook
(module-scope cache + listeners, same shape as useModules) resolving
`import.meta.env.DEV || (isAdmin && dev_mode)`. The sidebar item gates
on requiresCondition: 'devMode' and the route on the same hook, so a
visible nav entry can never redirect to the dashboard. The guard waits
on `resolved` first, or an admin deep link would bounce before the
dev_mode fetch lands. Settings' toggle calls refreshDevMode() so the
sidebar reacts without a reload.
Webhooks: the inbound receive/verify/forward console is server
configuration, not a daily ops surface, so it becomes a Settings ->
Admin tab next to the outbound subscriptions it complements. /webhooks
and /secrets/webhooks redirect there; the sidebar item is gone, which
empties ADVANCED_ITEM_IDS down to the queue console. The command
palette entry drops its navId (a Settings tab has no nav item for the
workspace permission gate to consult) and settingsIndex.js gains the
three entries check-settings-index.mjs requires for a new tab. The
native confirm() on delete becomes the shared useConfirm() dialog.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The retry added in 1f2381c behaved exactly as designed and still failed the
job: Hub was unreachable for the full two minutes ("Client.Timeout exceeded
while awaiting headers" on every attempt), not flaky. Retrying a single
registry harder cannot survive that registry being down -- it only delays the
failure. Two consecutive runs were blocked this way.
Fall back to Google's public Hub mirror, a separate failure domain, and re-tag
to the original name so the run step needs no changes (`docker run` resolves
the local image; default pull policy is "missing"). Hub stays primary.
Ref construction differs by image kind -- `mirror.gcr.io/library/<img>` for
official images, `mirror.gcr.io/<ns>/<img>` for namespaced ones -- and all
seven matrix images were confirmed to resolve on the mirror via
`docker manifest inspect`, opensuse/leap included.
ONLY the pull falls back. Wrapping `docker run` would re-run the suites on
failure too, quietly turning a flaky TEST green.
Verified locally end-to-end against the step text extracted from this YAML,
under `bash -e`, with a docker shim reproducing the runner's exact Hub error:
* Hub down, mirror up -> 3 warnings, REAL pull from mirror.gcr.io, re-tag,
and `docker run <image>` then starts a container with the repo mounted
while Hub is still blocked
* both down -> 6 warnings, one error annotation, exit 1 (fails honestly)
* Hub healthy -> exits 0 on the first pull, no mirror attempt, no noise
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`docker run` pulls implicitly, so a registry timeout ("context deadline
exceeded" reaching registry-1.docker.io) killed the whole matrix leg with exit
125 before a single test ran, and cost a manual re-run. Seven images per matrix
run makes that a recurring tax rather than a one-off.
Pull up front with backoff (10/20/30s) and only then run the suites.
ONLY the pull is retried. Wrapping `docker run` would re-run the suites on
failure too, which would quietly turn a flaky TEST green -- the opposite of
what this job is for.
Left test-system-utils.yml alone: it uses a job-level `container:`, which the
runner pulls itself, so there is nothing to wrap.
Verified against the step text extracted from the YAML, under `bash -e` (the
shell GitHub actually uses for `run:`): registry down -> 3 retry warnings then
one error annotation and exit 1; flaky -> recovers and exits 0; healthy ->
exits 0 immediately with no retry noise.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
backup_current tested `[ -f serverkit.db ]` FIRST and only fell through to the
PostgreSQL branch when no file was there. An install migrated SQLite ->
PostgreSQL keeps its old serverkit.db, and both deploy paths copy it forward on
file-existence alone (deploy_source, deploy_release), so the stale file is
carried into every future slot and never ages out. Such a box backed up the
abandoned file, skipped pg_dump entirely, and left PRE_UPGRADE_DUMP unset --
which migrate_database reads as "${PRE_UPGRADE_DUMP:-}", so its auto-restore
silently did nothing. PostgreSQL is the one engine that migrates IN PLACE and
cannot roll back by switching slots, so that dump is its only way home: the
exact safety net a61eab1/b48256f added was disabled for it.
migrate_database has always keyed off DATABASE_URL (its sqlite check). Test the
declared engine first so the two functions agree.
Deliberately NOT `grep DATABASE_URL=sqlite && [ -f db ]` as the reviewer
suggested: backend/config.py defaults DATABASE_URL when absent, so any install
whose .env lacks the line would fall to the else branch and update with NO
backup at all -- trading a mismatched backup for none. An explicit `postgres`
URL is a positive signal; the absence of a `sqlite` one is not.
Also:
* name the engine when it is one we cannot dump. "No SQLite database ..."
read like a benign nothing-to-do on a MySQL install that in fact had no
safety net. Prints the scheme only -- a DATABASE_URL carries a password and
this text lands in the update log.
* drop the stale "Python 3.11/3.12 is guaranteed by pre-flight" comment,
contradicted by locate_sqlite_python two lines below it (664a5e6).
* discord-notify: skip cleanly when DISCORD_WEBHOOK_URL is absent. Secrets
are not exposed to `pull_request` runs from a fork, so a merged fork PR
failed the job on an empty curl URL. Not switching to pull_request_target
for this -- handing a write-scoped token and the webhook to an
outsider-triggered run is a poor trade for a chat notification.
Tests: a PostgreSQL install carrying a leftover serverkit.db must still dump
and set PRE_UPGRADE_DUMP (fails on the parent commit, rc=1); an undumpable
engine must be named without leaking the password. Neither needs python3, so
both run on all 7 images. Workflow verified end-to-end against a synthetic
event payload: fork PR exits 0 without calling curl, normal PR still builds the
same embed.
Verified on the full CI matrix (ubuntu 22.04/24.04, debian:12, rockylinux:9,
almalinux:9, fedora:40, opensuse/leap:15.5), all six suites: 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
copy_sqlite_db and verify_sqlite_db reused locate_python, which enforces
3.11 <= v <= 3.12. That bound is correct for rebuild_virtualenv and
preflight_check, but snapshotting or integrity-checking a SQLite file needs
nothing beyond the stdlib sqlite3 module. Reusing it coupled the DATA-safety
paths to the venv-builder constraint and blocked updates for a reason unrelated
to the data:
* an all-Docker install returns before preflight_check ever runs and does
every real step in the container -- yet backup_docker_db snapshots with the
CONTAINER's python and then verified the extracted file with the HOST's.
On a stock EL9 (python3.9) or Ubuntu 22.04 (python3.10) host that halted
with "Could not obtain a VALID database backup", blaming the backup for a
missing interpreter.
* backup_current halts when it cannot snapshot, so those hosts could never
update at all.
Split out locate_sqlite_python: any candidate that can `import sqlite3` wins,
and the install's own venv interpreter is preferred -- on a real box it is
guaranteed present, guaranteed to carry the module, and independent of PATH.
locate_python keeps the 3.11/3.12 bound where it belongs.
Surfaced by the scripts-ci unit matrix: almalinux:9 and rockylinux:9 ship
python3.9, so the suite's own probe accepted an interpreter locate_python
rejected, and copy_sqlite_db/verify_sqlite_db failed there while passing on
python3.12 runners.
T10i pins the contract: on a box where every candidate reports 3.9 (locate_python
fails), the helpers still resolve an interpreter, prefer the slot's venv, and
round-trip 100 rows. Fails on the parent commit, passes here.
Verified on the full 7-image CI matrix (ubuntu 22.04/24.04, debian:12,
rockylinux:9, almalinux:9, fedora:40, opensuse/leap:15.5), all six shell suites:
0 failed. test_update is 63 passed / 0 failed on every image carrying a python3.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
verify_pg_dump only runs its structural `pg_restore -l` TOC check when the
binary is on PATH. The harness stubs every other external command it touches
(systemctl, nginx, npm, curl, docker, flask, pg_dump) but never pg_restore, so
two assertions were decided by the host:
* no postgresql-client -> branch skipped, check degrades to "non-empty",
and the synthetic `PGDMP fake contents` fixture passes
* postgresql-client present -> the real binary parses the version bytes
after the PGDMP magic, rejects the fixture, verify_pg_dump returns 1
That is the CI failure. T10f failed directly; T10e failed downstream because
backup_current correctly halts on an unverifiable backup. update.sh is right --
the tests leaked.
Add a make_stub_pg_restore factory mimicking the real -l semantics (rejects
missing/empty, rejects anything without the PGDMP magic, else prints a TOC) and
install it in T10e/T10f. Give T10f a non-empty non-archive fixture too: that is
the case separating the two branches inside verify_pg_dump -- the size check
passes it, only the TOC listing catches it -- so the path the test was named
for is now actually exercised.
Verified both with and without a pg_restore on PATH: 59 passed, 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A backup that was never verified is a hope, not a safety net. Closes the
remaining ways the pre-upgrade safety net could silently not exist:
- Every backup is verified immediately after creation: SQLite snapshots via
PRAGMA integrity_check, PostgreSQL dumps via size + pg_restore TOC
listing (when available). Invalid artifacts are deleted and the update
halts — a malformed backup is never mistaken for a good one.
- Partial artifacts from a failed backup are removed (rm on failure), so a
half-written file can never be 'restored' later.
- A failed IN-PLACE PostgreSQL migration now automatically restores the
verified pre-upgrade dump (pg_restore --clean --if-exists) before
halting; without it the half-migrated schema would stay live against
the old code. If the restore itself fails, the halt message carries the
exact manual pg_restore command.
- backup_docker_db tries both python and python3 in the container, and now
verifies the result too — the plain docker cp fallback can no longer
pass off a torn copy as a backup.
- Backup retention capped: default 5 (SERVERKIT_BACKUP_RETENTION
overrides) across trees, .db and .dump files. Full-size DB copies pile
up fast — a real 25G droplet carried 6.3G of stale backups.
Tests: T10f (verification helpers accept/reject correctly), T10g (failed
in-place PG migration auto-restores with the exact pg_restore args),
T10h (retention prunes all three backup kinds to the configured cap).
Postgres panels updated with zero safety net: backup_current found no
SQLite file and just warned, while the migration runs IN PLACE against the
shared external DB — a failed migration left no way back.
- backup_current now pg_dumps (custom format) when .env points at
PostgreSQL, sanitizing SQLAlchemy driver suffixes (postgresql+psycopg2://)
that libpq can't parse.
- Same rule as SQLite: no valid backup, no update — halt when pg_dump is
missing (install postgresql-client) or the dump fails.
Tests: T10e covers the dump invocation + URL sanitization and the
halt-on-failed-dump path.
The venv binding check only matched the quoted VIRTUAL_ENV="/path" form;
venvs created on some platforms write 'export VIRTUAL_ENV=/path' unquoted,
so the binding read as 'unknown path' and always triggered a rebuild.
Now matches both forms (and skips cygpath-style indirection), so fresh
installs keep the fast prebuilt-venv path.
Juan Denis