Plan 55 task 6, backend half. Installing a plugin's requirements runs pip with
the backend's privileges, and a setup.py hook is arbitrary code, so it is
opt-in and off by default -- correct, and the part that was missing is that
nothing ever said so. The skip was a logger.warning: the requirements.txt was
written next to the extension and the panel showed nothing, so an extension
could sit there running with imports it does not have and the operator had no
way to find out short of reading the install log.
pending_requirements() answers it: whether a file is pending, its contents, its
path, the parsed package lines, and the exact env var that changes the
behaviour. GET /plugins/<id>/requirements exposes it, admin-only.
Strictly read-only. Nothing here installs anything -- the plan is explicit that
this surface introduces no auto-install behaviour change -- and a test pins
that by failing if _install_requirements is reached.
The env check the installer inlined is now plugin_pip_enabled(), shared by both
sides. If the surface and the installer ever disagreed about what "enabled"
means, the page would describe a behaviour the installer does not have.
Two honest edges: the content is capped at 64KB before being echoed into a
browser, and pip_enabled is reported alongside pending, because a file written
while the opt-in was off survives turning it on -- the surface reports both
facts rather than guessing which applies.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 55 task 7, backend half. A manifest's `permissions` array was
declaration-only: the install dialog showed it and nothing ever compared it
against what the extension went on to do.
The capability gate now records every call through it -- including the
refusals, which are the interesting ones -- so `usage_report(slug)` can answer
declared-versus-observed per extension. Observations are in-memory and
per-process by design: the question is "what has this extension done since the
panel started", and persisting it would outlive the evidence and start reading
like an audit trail, which this is not. `has()` deliberately does not record; it
is a question, not a use, and counting it would fabricate activity for callers
that only checked.
The larger half of this change is admitting what the panel cannot see. Task 7
was specified as hooks at "the shell/docker/db helper entry points", and those
do not exist: the SDK exposes no shell, docker, filesystem or network helper at
all, and hands out `db` as raw SQLAlchemy. An extension wanting any of them
imports the host module directly, where no in-process gate is involved. The one
genuinely gated capability is `agent.command:<action>`, because the SDK is the
only way to reach an agent.
So observability is data, not prose: OBSERVABLE_PREFIXES marks what the gate
truly mediates, and the report marks every other declared permission
`observable: false` rather than showing it as "never used". Absence of evidence
is not evidence of absence here, and a UI claiming otherwise would be inventing
enforcement the panel does not have -- exactly the honesty the plan's D2
decision asks for.
GET /plugins/<id>/permissions returns the report, admin-only: it describes what
third-party code has been reaching for.
The UI surface and the docs follow.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 72 item B.1. Every redeploy path reached its point of no return -- a
compose down, or a stop+remove -- before anything had checked that the new
version could actually run. An invalid compose file, an image that no longer
pulls, or a port somebody else took stopped the site and left it stopped,
nginx 502ing until a human intervened. The fix is not blue/green (that is
B.2/B.3, deliberately not attempted here). It is ordering.
deploy_preflight_service runs everything checkable while the old containers
keep serving: docker compose config --quiet -- wiring the long-existing
validate_compose_file, whose only caller until now was a manual API endpoint
-- then image resolution to completion, then host port conflicts. Nothing in
it stops, removes, recreates or rewrites anything; its only side effects are
pulls and builds, which are additive. Each caller aborts on a blocking
finding, and the log line says the running deployment was left untouched,
because that is the whole user-visible payoff.
Three paths were unsafe and are fixed; each now carries an explicit marker at
the line past which the live deployment is gone.
The single-container path had the sharpest version of the bug: the registry
pull sat BELOW the stop and remove, so a registry hiccup left the app with no
container at all. The build already ran earlier, so the pull is the only
pre-traffic requirement, and it now happens first. The template update
renders the new compose to a sibling dotfile and validates and pulls THAT, so
the live docker-compose.yml is untouched until validation passes; same
directory, so relative bind mounts and .env resolve identically. The git
webhook default builds before it stops, and its later up --build is a cache
hit.
Everything is fail-open, which matters more than the checks themselves: when
compose config cannot be introspected the dependent checks are skipped, and a
pull failure is downgraded to advisory when every referenced image is already
present locally, so a locally-tagged image that was never pushed keeps
deploying exactly as before. A preflight must never abort a deploy the old
code would have run.
The port check needed reframing rather than reuse. The probe itself is
ManifestApplyService._port_bound, called through the class so the existing
test seam serves both surfaces, but a bare probe flags the deploy's own live
container and would abort every ordinary redeploy. It only fires for ports the
new config wants that the running project does not already publish, and skips
entirely when ownership cannot be determined. It is deliberately not applied
to the single-container path, where app.port is held by the app's own
container when one exists and there is nothing live to lose when one does not.
Paths already safe were left alone rather than given ceremony: the
zero_downtime restart builds first and never downs, template install has no
live container, and the upload/rollback/image-update paths already pull or
build before compose up.
Also fixes a pre-existing bug found in update_app: install_info was assigned
only inside a try but read unconditionally later, so a missing or corrupt
.serverkit-template.json raised NameError *after* compose_down and left the app
stopped with a new compose that was never started -- the exact outage this
item targets.
No existing test asserted the old unsafe ordering, so nothing had to be
rewritten. The new tests prove the ordering directly: on a validation failure
compose_down is never called, the template's compose file is byte-for-byte
unchanged, and stop_container/remove_container are never reached on a pull
failure.
Not verified: no real Docker was exercised -- every docker seam is stubbed, so
build/pull exit behaviour on a mixed project is reasoned-about, not observed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 72 item A.2. FleetDoctorService, FleetRepairService, the FleetDoctorResult
model, migration 063 and their tests were all complete and reachable from
nothing: no routes, register_jobs() never called, no seeded schedule. The
frontend was further along than the backend -- getFleetDoctorReport() and
runFleetSweep() have been calling /doctor/fleet and /doctor/fleet/run, two
endpoints that did not exist, so the multi-server health story the agent
platform advertises 404'd on contact.
Nothing in those services was redesigned. The one addition is a read-side
aggregator: they could run a sweep and persist rows but had no way to hand
back a per-server report, so fleet_report() composes one. It lists every
registered server rather than only connected ones, which is what makes an
unreachable box a reported state -- last known rows plus connected: false --
instead of a hole in the report. Persisted rows key on check_key while the
UI's field-driven renderer keys on key, so the report emits both.
Three admin-only routes, matching api/doctor.py's gating exactly. The sweep
returns 202 with a job id and never runs on the request thread: it fans out
across agents over the network, and the agent gateway registry is
single-worker and in-memory. Repair is a thin passthrough to FleetRepairService,
which already owns the allowlist, capability gate and audit trail; refusals
come back as per-item results inside a 200, so one offline server cannot fail
a batch. The batch is bounded at 25 so a single request cannot fan an
unbounded number of remote mutations at that single worker.
The schedule is daily like the host doctor but starts 30 minutes behind it,
so the fleet fan-out contends with neither the local sweep nor the boot-time
agent reconnect storm.
The frontend gets a Fleet Doctor section rather than losing its dead methods.
The per-check article was extracted into one component now shared by the host
and fleet lists, so fleet findings render through the same
repairable/repair_ref path instead of a parallel implementation; the confirm
modal says plainly that a fleet repair acts on a remote server through its
agent and is audited.
test_jobs' builtin schedule count moves 21 -> 22 for the new schedule. The
builtin.* kind count is untouched -- this kind is doctor.fleet.run.
Note for anyone reading the suite: tests/test_fleet_doctor_dns_rotation.py is
skipped at module level and proves nothing here. Its rotating-DNS-window API
did not survive the recovery rebuild; that is pre-existing and unrelated.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to 07dbffbe, which took the wrong branch. That commit noticed
verify_run needs a remote copy and responded by withholding the Repair button
from local-only backups. It also broke test_doctor_backup_proof_matrix, which
asserts every unverified policy gets a repair_ref -- and that test was right.
Most installs have no offsite storage configured, so hiding the button there
means the check that tells you whether your backups can be trusted offers a
fix to almost nobody.
There were always two verification ladders and only one was wired. A run with
an offsite copy is verified against the provider by verify_run. A local-only
run is verified in place by backup_verify_service.verify_run_tier1 -- read the
archive, checksum it against the stored manifest, set the run's level to
listed or hashed. That function was complete, tested and reachable from
nothing: it had zero callers in application code. The repair now picks the
ladder from what the run actually has, so the button works either way and the
check goes back to offering it unconditionally.
Tier 1 mutates the run's verify ladder without committing, so the commit is
the repair's -- otherwise the level is discarded and the next doctor sweep
reports the same backup unverified again. A test pins that.
Both local failure modes are reported as failures rather than fixes, matching
how the remote mismatch is already handled: level 'none' means the archive
could not be read at all, and a 'listed' level carrying a verify_error means
it is readable but is not the archive the manifest describes. Either is worth
knowing before a restore depends on it, and neither is a repair.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 72 item A.1. The backup_drill_stale and backup_unverified checks both
emitted repairable: True, but DoctorService.repair() only knew drift, service,
dns and extension. Every Repair button those checks rendered came back
"Unknown repair kind: backup_drill" -- a button that failed 100% of the time,
on the check family whose entire job is telling you whether your backups can
be trusted.
The dispatcher now handles both, and they are deliberately asymmetric because
the underlying services are. request_drill enqueues a backup.drill.run job, so
that answer carries the job id for the console to link to. verify_run does the
size and checksum comparison inline and returns a verdict, so there is no job
id -- claiming one would send the console looking for a job that does not
exist. The plan assumed both enqueued; only the drill does.
Verification finding a BAD copy is reported as a failure, not a fix. It is a
worse outcome than "unverified" and calling it a successful repair would be a
lie told by a green checkmark. Both branches turn their services' ordinary
refusals -- a drill already in flight, no successful backup to drill, backup
not found -- into plain error text rather than letting them escape, and a
policy_id echoed back for a since-deleted policy reads as gone instead of
raising.
The second half is the same defect from the other side. verify_run compares
against the REMOTE copy and raises when there is none, so a local-only backup
was rendering a Repair button that could never succeed. That check now only
offers the button when there is something to verify against, and otherwise
says what would actually fix it: send the policy offsite, or run a restore
drill.
Drills started this way are attributed to 'doctor' rather than 'manual', so
the drill history says where they came from -- the field is free-form and only
displayed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 73 item 8. The installer opened 80/443 on whatever firewall it found and
shrugged -- "assuming ports 80/443 are already open" -- when there wasn't one,
which is the state of a stock Ubuntu/Debian VPS. That box then ran ServerKit
with every port a deploy maps onto the host (8001-8999, plus database
containers on their stock ports) facing the internet. fail2ban only shipped
with the "full" capacity profile, so the typical small VPS got neither.
The dangerous half is enabling default-deny without allowing the port sshd
actually answers on, over the SSH session running the installer. detect_ssh_ports
therefore never guesses: it asks `sshd -T` (the effective config, which resolves
the Include that Ubuntu 22.04+ ships), then sshd_config plus its drop-ins with
comments stripped, then a live `ss`. Multiple Port lines all come back. 22 is
used only when a config exists and names no port -- sshd's compiled-in default
-- and if nothing answers at all the function fails and no firewall is
installed. An exposed box is recoverable; a locked-out one usually is not.
That interlock had a real bug in its first draft, caught before it shipped and
now pinned by a named regression test: most boxes have no sshd_config.d/, the
unmatched glob makes cat exit non-zero, and under `set -o pipefail` that wiped
a correctly parsed answer and fell through to the 22 default -- returning 22 on
a box whose sshd is on 2222, which is precisely the lockout this function
exists to prevent.
Ordering is the other safety property and is asserted: every allow rule (all
detected SSH ports, 80, 443, and PANEL_PORT when non-standard) lands before
`default deny incoming`, and the policy before `--force enable`. The bootstrap
refuses to act inside a container, when a firewall is already active (it is the
operator's -- opening ports on it is fine, rewriting its default policy is not),
under SERVERKIT_SKIP_FIREWALL=1, and when a previous run bootstrapped but the
firewall is off now, since the operator turned it off deliberately. Non-Debian
installs are byte-identical to before: RHEL-family boots with firewalld already
active, and a firewalld default-deny bootstrap for boxes where it is inactive
is marked as an explicit follow-up rather than guessed at.
fail2ban moves to the standard profile with an sshd jail that watches the same
ports detect_ssh_ports found -- a jail pinned to :22 on a box listening on :2222
bans nobody -- and declares `backend = systemd` when there is no auth.log to
tail, without which the jail refuses to start and fail2ban looks installed while
protecting nothing. certbot stays full-only.
setup-health gains firewall (critical) and fail2ban (recommended) items. The
split is deliberate: this file grades critical by silent breakage, and an
unfirewalled box qualifies -- the operator set it in motion by deploying,
nothing in the UI says the port is public, and the failure mode is compromise
rather than an error. A missing fail2ban thins defence in depth but breaks
nothing silently, since the panel already throttles logins per IP. Both are
gated on actually owning the host -- Linux, /etc/serverkit present, not
containerised -- so a dev checkout or CI runner drops them entirely instead of
reporting a fake ok that would inflate the score.
SetupHealthWidget was orphaned; so, it turns out, was its stylesheet, which
main.scss never imported. Both are wired up, as a plain admin-only card above
the board rather than a placeable widget: an unfirewalled box has to be visible
without the operator first opting into showing it. It collapses to one line
when everything is clean.
Implementing this inverts a shipped test that asserted provision_hardening was
a no-op on the standard profile; it is rewritten to the new contract rather
than deleted, with a minimal-profile case added alongside.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 73 item 5. The Domains SSL modal asked for a contact email on every
single certificate, and the backend hard-required one per call. The app-detail
Settings tab did remember it -- in localStorage, so the memory was per browser
and invisible to the other surface. The two never agreed, and neither could
help the paths with no form at all: domain attach passed `admin@<host>` when
it had nothing else, an address that usually does not exist, which is where
every expiry notice for those certificates went to die.
There is now a single panel-wide ACME contact in system settings.
get_acme_contact() resolves an explicit address first and falls back to the
stored one; remember_acme_contact() writes it back whenever a request carries
an explicit address. Both are best-effort on the settings side by design: a
settings failure must not break issuance, and failing to remember an address
must not turn a successful certificate into an error. An empty input never
clears a stored contact, since the automatic paths pass whatever they were
given and that is frequently nothing.
Wired into every issuance entry point -- the domain SSL enable route, the
generic certificate route, and domain attach, which now consults the stored
contact before falling back to the admin@<host> guess. The address is only
remembered after issuance actually succeeds; remembering one Let's Encrypt
rejected would be worse than remembering none.
Prefill needed its own endpoint. The settings routes are all admin-gated, but
issuing a certificate for an app you can edit is not an admin action, so
GET /ssl/acme-contact is jwt_required only -- it returns an address the
operator chose for notices, not a credential. The Domains modal and the
Settings tab both prefill from it, and the Settings tab keeps its localStorage
seed so a browser that already has the value shows it without a flash.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 73 item 7. Let's Encrypt prefers IPv6 whenever a AAAA record exists, so a
domain whose A record points at this server perfectly still fails HTTP-01
issuance when its AAAA points somewhere else. That shape looked healthy
everywhere in the panel, which is what made it expensive: the operator saw a
verified domain, a green doctor report, and a certificate error naming nothing
either of them had mentioned.
Both DNS surfaces were blind to it for different reasons. The doctor's sweep
returned ok as soon as the server IP was *among* the resolved addresses, so
the extra IPv6 answer never registered -- and that branch is reached precisely
when the A record is correct, i.e. exactly the case worth reporting.
verify_domain used socket.gethostbyname, which is IPv4-only and cannot see a
AAAA record at all.
Both now compare every resolved address against this server: the configured
public IP plus the host's own interface addresses, so a correct dual-stack
setup stays quiet and only a genuine stray is named. Comparison is by parsed
value rather than by spelling, since an expanded 2001:0db8:... form and its
compressed equivalent are the same address, and psutil's zone ids (fe80::1%eth0)
are stripped first. Interface enumeration happens once per doctor sweep
instead of once per domain.
The doctor reports this as a warn, not a fail: the site itself is reachable
and it is a stray record rather than a broken one. Both messages say what to
do -- remove the AAAA record or point it at this server's IPv6 address --
rather than only naming the symptom.
verify_domain keeps its existing response shape. ip_address still carries a
single IPv4 address for the callers that predate this, with the full set added
alongside as ip_addresses, plus aaaa_conflict and a warning string. The
Domains page needed the last part to matter: it printed a green "Domain
verified!" toast for anything that resolved, so a conflict the API now
reports would have gone straight back to being invisible.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 73 item 9. Three blind spots with one theme: the panel trusted a signal
that did not mean what the UI claimed it meant.
The doctor's service sweep only asked `systemctl is-active`. nginx keeps
reporting active while serving a config it can no longer parse, so a box that
was one `nginx -t` away from explaining itself got a clean bill of health. A
new nginx.config check runs that test and fails with the file and line nginx
named. It is deliberately not repairable: config repair is operator territory
and a wrong guess takes every site on the host down. A missing nginx binary
warns rather than fails, since a profile without nginx is legitimate.
NginxService.restart now tests the config first, the way reload always has.
The asymmetry is what made this worth fixing: a reload against a broken config
fails safe -- nginx rejects it and keeps serving the old one -- while a restart
stops nginx before starting it, so the same broken config becomes a full
outage with nginx dead until someone fixes it by hand. Testing first turns
that outage back into an error message.
get_services_status substring-matched psutil process names while the
start/stop/restart buttons went through systemctl, and the two disagreed in
both directions. `mysql` matched the process `mysqld`, so a MariaDB box whose
unit is `mariadb` showed a running `mysql` row whose buttons systemctl could
not honour; `docker` matched `dockerd` and `redis` matched `redis-server`,
listing one daemon twice and three times; and any unrelated process whose name
merely contained a service name reported that service up. It now asks systemd
-- the same authority the control path uses -- so the list cannot claim a state
the buttons are unable to act on.
That query is one `systemctl show` for the whole list rather than one call per
unit, because this backs a polled list. Units the host does not have are
dropped instead of shown as stopped: MONITORED_SERVICES carries several
spellings of each daemon so whichever one a distro uses is found, and the
misses were noise. Aliases resolving to the same unit collapse to one row via
the Id property. If systemctl is absent, times out, exits non-zero, or returns
a block count that does not match the units asked about, the old process-name
heuristic still answers rather than the list going silently empty -- and it
remains the path for non-Linux dev boxes, which have no systemd.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 73 item 3. The quick-access rail links the local target at /etc/nginx,
but /etc was never in ALLOWED_ROOTS, so the shortcut returned 403 for every
user including admins -- a dead button, and our own regression: it shipped
after the review that flagged the surrounding SSL story.
/etc/nginx is now an allowed root, but browse-only. The panel generates those
vhosts itself, so a hand edit is silently overwritten on the next domain
change, and a writable /etc/nginx would hand anyone holding files.write a
root-owned config that the panel itself reloads -- a much larger grant than
"let me look at the effective config", which is all the shortcut is for.
READ_ONLY_ROOTS is a second tier under ALLOWED_ROOTS rather than a special
case for this one path: is_path_writable() = allowed AND not read-only, and
every mutating entry point now gates on it instead of on is_path_allowed, so
adding a future root to the list is enough to make the whole write surface
refuse it. Both comparisons resolve with realpath first, so a symlink planted
in a writable root cannot launder a write into the subtree.
copy is deliberately asymmetric -- copying a vhost out to keep only reads the
source, so only the destination is gated -- while move removes its source and
therefore checks both ends. The upload endpoint repeats the check because it
writes to disk directly instead of going through FileService's methods.
Only this subtree is opened, not /etc: /etc/passwd and /etc/shadow stay
denied, and the panel-internal PROTECTED_ROOTS exclusion is untouched
(GHSA-rm3m-9mvw-68fh).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 73 item 1. SSLService hardcoded /usr/bin/certbot, so the install method
certbot itself recommends -- snap, which lands in /snap/bin -- produced a
FileNotFoundError the panel surfaced as an opaque failure. is_command_available
had the same blind spot: it probed /usr/bin, /usr/sbin and the two local dirs
but never /snap/bin, which is routinely absent from a service's $PATH.
resolve_certbot_bin() now tries the CERTBOT_BIN override, then shutil.which,
then the explicit candidate list including the snap path. Positive results are
cached (an install location does not move mid-process) but negative ones are
not, so installing certbot is picked up without a panel restart;
install_certbot clears the cache outright. CERTBOT_BIN as a class attribute is
gone -- it captured the value at import time, which is exactly why an override
set later never took effect -- replaced by certbot_bin() plus CERTBOT_FALLBACK
so a command built on a host with no certbot still fails cleanly.
The second half is the failure the reviewer actually hit on stream: with
certbot present but its nginx plugin missing, `certbot --nginx` dies deep
inside certbot with "the nginx plugin is not working / binary doesn't exist",
and we passed that through raw. obtain_certificate now preflights -- nginx on
the box, then `certbot plugins` for an enabled `* nginx` -- and returns an
actionable error naming python3-certbot-nginx before certbot is ever invoked
for issuance. A failing plugins probe falls back to the distro package check
rather than blocking issuance on a host where the plugin is clearly present.
AdvancedSSLService's wildcard and multi-domain commands go through the same
resolution instead of the bare name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
.gitattributes pins the whole repo to `text=auto eol=lf`, but plugin_service
wrote the tracked manifests through Python's text mode, which translates \n to
\r\n on Windows. Any install on a Windows dev box therefore rewrote
frontend/src/plugins/*/plugin.json with CRLF, and the backend suite -- which
installs builtin extensions -- left four modified manifests behind on every
run. The diff showed no content change, which made it look like unrelated
churn and made it easy to sweep the files into an unrelated commit.
This is the same failure the comment above the write already documents: the
trailing-newline half was found and fixed earlier, the line-ending half was
missed. Both halves now live in the same explanation.
_regenerate_frontend_manifest's write gets the same treatment for consistency,
though nothing covers it -- it is skipped under the testing config so it cannot
be reached from a test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review catch on #97. The chat connection test failure log interpolated
DeliveryResult.error verbatim. _deliver_webhook builds that from str(exc) on a
network error, and requests embeds the full URL in its exception text -- so the
line could carry a Discord/Slack/Teams webhook token or a Telegram bot token,
all of which live in the URL *path*. to_dict() masks `destination` even from an
admin, and /api/logs serves the log files back to the panel, so the log line
leaked past the API's own masking.
Dropping the reason would have restored the earlier problem of a failed test
button leaving no diagnostics anywhere, so it is redacted instead: URLs keep
scheme and host and lose the path, and bare bot tokens are substituted. The
host is what makes the line worth keeping and is not a secret.
Also normalise ChatWebhookConnection.raw_credentials() to {} when the stored
JSON is well-formed but not an object. A hand-edited or corrupt row could hold
`[]`, which every caller then treated as a mapping -- credentials() called
.items() on it, so to_dict() and therefore the list endpoint raised before the
write path was even reachable. Guarding at the source covers all callers and
matches the existing categories() pattern.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to PR #94. ChatWebhookService.update() decrypted every stored
credential and re-encrypted the lot on any credential-bearing request.
decrypt_secret_safe() returns its input unchanged when a value will not
decrypt, so a wrong-key or not-yet-migrated row got wrapped a second time
and became unrecoverable -- and it happened to fields the request never
named. It now patches the ciphertext dict directly and encrypts only what
it assigns, so untouched credentials survive byte-identical.
Also tolerate an unchanged is_default in a round-tripped body (to_dict()
always emits it, so GET -> edit -> PUT rejected an untouched field), and
record the delivery failure reason in the server log. The reason stays out
of the HTTP response: transport errors embed the full webhook URL and any
Telegram bot token.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This is the change behind the headline symptom. A DevTools capture of /domains
showed ~70s of network activity and a 404 taking 28.77 SECONDS. The backend
measured that same request at 1.0ms in its own api_usage_logs table. A routing
404 runs no handler and touches no database, so it cannot spend 28s working —
the entire gap was Stalled/Queueing in the browser.
The cause: in dev the API was same-origin with the Vite dev server, which serves
each of the ~760 source files as its own request. A browser allows six
concurrent HTTP/1.1 connections per origin, so twenty API calls queued behind
hundreds of module requests. Splitting the origin doubles the usable sockets
and, more to the point, stops a slow API call from blocking a JS module.
Same-origin was not an arbitrary choice — the comment it replaces warned about
"CORS/preflight redirects". Both halves of that are handled:
- CORS: config.py DEFAULT_CORS_ORIGINS already lists the Vite dev origin on
both localhost and 127.0.0.1, so nothing new is being allowed.
- Preflight: every panel request carries an Authorization header, which makes
it non-simple. Without Access-Control-Max-Age the browser re-runs the
preflight every few seconds (Chrome caches 5s by default), so a cross-origin
panel would pay two round trips per call forever — worse than what it
replaces. CORS now sends max_age=3600, verified against the real create_app()
rather than assumed: a preflight from http://localhost:41921 returns 200 with
Access-Control-Max-Age: 3600 and the matching allow-origin. That header is
worth having regardless, since any deployment serving the panel from a
different origin than the API was paying the same tax.
Escape hatch: VITE_API_PROXY=true restores same-origin for a setup with an
origin CORS does not expect. Socket.IO is deliberately untouched — it stays on
window.location.origin in dev, because a WebSocket upgrades out of the HTTP
connection pool and gains nothing from the split.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Incidents was showing the same idea twice: a converged "All incidents" table, and
under it a second table headed FLEET ALERTS with its own Active/Acknowledged/
Resolved segment row. A host alert IS an incident whose source is a host alert —
the SOURCE column already said so. FleetAlertsPanel is deleted (188 lines, and
Incidents was its only importer); alerts are folded into the existing row merge
with server -> subject, metric/value/threshold -> what happened, severity ->
impact, status -> state. Ack and Resolve move into the row-actions cell, shown
only for rows that have an alert id and are not resolved. The segment became an
"Acknowledged" builtin view alongside the Active/Resolved ones that already
covered its other two buckets.
Config Templates: the Compliant/Drifted/Unknown KPI band and its progress bar are
gone, replaced by a per-row Compliance column plus Drifted/Compliant builtin
views. The band could say three assignments had drifted but never which template
caused it; the column answers that per row. This needed the template payload to
carry its own tallies, which is the same single query the bare assignment count
already cost.
Recycle bin: the All / domains / saved-views segment row is gone — those buckets
are a filter on the Type column, and the Type column's own menu does it with live
per-value counts and can be saved into a view. Settings tab panes now sit on the
same card surface every other table in the app does.
Users: Add User and Invite User move onto the chrome line next to the filter and
"..." instead of each occupying a row of their own above its table.
SSL certificates: was a card list with a KPI band, one tab away from /domains
being a full table. Now a table with saved views (Expiring soon / Valid / Failed
or missing), search, column filters and export. The leading tile takes the row's
state too — a fixed green shield beside an expired certificate contradicted the
Status cell next to it.
Service detail: the Environment and Events tabs become tables with the inline
chrome an inner tab gets.
Fixes found by verification, in code this round touched:
- ServerTemplates had four columns with a `value` accessor and no `render`, so
DataTable fell back to row[key]: two rendered blank, one printed a raw array,
and `services` printed a list of OBJECTS, which React refuses to render — that
one took the whole tab down for any template declaring a service. Pre-existing,
and invisible because the screenshot fixture sent `services: ['sshd']` as
strings. The fixture now matches to_dict() so it cannot hide it again.
- useTopbarChrome returns a ready `actions` value. All eight hoisting pages
destructured only `portal` and dropped the fallback, so outside a tab group the
filter button and "..." would have gone nowhere rather than inline.
- sameViewState compares only the `page` keys a page still captures. Retiring a
key (as the recycle bin just did) otherwise stranded every view a user had
already saved with it, pinning "Unsaved" on with no way to clear it.
- The environment tab's "Applies to" column was gated on the app's CURRENT
compose services, so a variable targeting a since-renamed service showed
nowhere and read as applying to everything.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 70. Domain went through this and TEN read paths were wrong, four with
effects outside the panel. Application has 211 call sites across 68 files and
drives more -- container lifecycles, nginx, backups, registry pulls, the OS
crontab -- so it was audited in full first (docs/plans/70_...md). 180 sites are
now query_active(); the 35 left unfiltered are each commented in place.
**The delete / purge split is what makes the tombstone honest.** delete_app used
to run compose_down(volumes=True) and rmtree(). Keeping a tombstone AND doing
that would put a Restore button next to a row whose data the delete had already
destroyed -- the one promise a recycle bin cannot break. So:
DELETE containers down (VOLUMES KEPT), vhost removed, firewall closed,
cron suspended, row tombstoned. All reversible.
RESTORE vhost rewritten, firewall reopened, cron resumed. Containers left
STOPPED -- "put it back" must not silently start processes or open
ports, the same reasoning that stops domain restore re-issuing certs.
PURGE volumes removed, uploaded source deleted, cron detached.
register() gained on_purge for that, wired into BOTH purge paths. And
purge_expired had NO scheduler -- fine when a tombstone was just a row, not fine
when it holds data volumes and a source tree, so a daily builtin job now runs
it. Without that, "kept until purge" would have meant kept forever and delete
would have quietly stopped reclaiming disk.
A side effect worth naming: the old _remove_data_flag protection for database
engines is now the DEFAULT for every app, and the flag is carried into
?purge=true so an explicit skip-the-bin delete still honours it.
**Cron was the blocker with no mechanical answer.** before_delete never fires on
a soft delete, and clear_application is LOSSY -- it nulls the association
without recording it, so nothing could put it back. suspend_for_application
instead disables only the jobs that were CURRENTLY ENABLED and stamps each
suspended_by_app; resume re-enables exactly those. That flag is the whole point:
without it a restore switches back on jobs the user deliberately turned off.
clear_application survives as purge-only, where lossiness is correct.
**The indirect reach (audit 3.1) is the class that grep cannot find.**
ContainerSleepPolicy, BackupPolicy, DeploymentJob and WafPolicy are enumerated
on a SCHEDULE and resolve application_id afterwards, so they never appear in a
search for Application.query. Left alone, a tombstoned app's sleep policy is
swept forever and wake_app restarts the containers the delete tore down. Those
loops test Application.deleted_ids().
Deliberately NOT scoped: reservation checks (port, name, directory) -- a
tombstone must keep holding those or a new app takes them and the restore is
refused; api/git.py's webhook VISIBILITY gate, which falls through to
"panel-wide visible" so hiding a tombstone would WIDEN access; and cron
attribution, which only labels a row.
Verified: 15 new tests, the migration replayed on a fresh DB (a tombstone frees
its private_slug, two live rows still collide), and a full delete -> restore
round trip. The 20 remaining full-suite failures are the known
serverkit-wordpress-not-installed class and are identical on a clean tree.
Two traps caught by testing, both recorded in the plan: a bad import made the
ENTIRE purge cleanup silently no-op while purge() still reported success
(_run_purge_hook swallows by design, so assert on the warning, not just ok), and
private_slug had to lose unique=True at the MODEL level or a models-built
database burns the slug regardless of the migration.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The two worst endpoints in the backend, both measured in plan 69 phase 6.
**/status/apps** ran one `docker compose ps` per app plus a `docker inspect`
per container -- roughly 30 apps x 3 containers = ~120 process spawns per
request -- and it is refilled on a timer, so the cost never amortised. It is
now ONE host-wide `docker ps` whose output is indexed by the compose
project/service/working-dir labels Docker already stamps on every container,
shared between callers through a 3-second in-memory snapshot.
The snapshot is deliberately module memory rather than CacheService: it must be
droppable synchronously, because a status that survives the action that changed
it reads as a broken panel. So `invalidate(app_id)` is called from start, image
apply, deploy and rollback. 3s is short enough that a missed hook self-corrects
within one poll.
**/admin/activity/summary** ran two 90-day loops issuing one COUNT per day =
180 sequential queries. Now one `GROUP BY func.date(...)` per series with the
gaps zero-filled in Python, so the response shape is byte-identical. `func.date`
is portable across the SQLite and PostgreSQL this app supports. A third N+1 fell
out on the way: the top-users block did one SELECT per user, now a single IN.
New tests assert the zero-fill specifically -- a day with no rows must still
appear and the series length must match the window, which is exactly what a
GROUP BY drops if you forget.
1140 backend tests pass. The 3 failures in that run (test_fail2ban_jail_service,
test_url_swap, test_site_routing) all reproduce on a clean tree -- the last is
the known "serverkit-wordpress extension not installed" class.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second endpoint after /apps, same pattern. Server.to_dict grows a `fields`
parameter so $select skips the derived work rather than only trimming the
payload: `onboarding_progress` is a JSON walk that is ~29% of a row and is read
by exactly one screen, and `group_name` is a relationship load. A caller that
needs {id, name} -- Domains and Backups both do -- now pays for neither.
`group_name` is eager-loaded when it IS wanted, so the list stops issuing one
SELECT per server for it.
Two details this route needed that /apps did not: it answers with a bare JSON
array, so no paging envelope is emitted (that would break every caller), and an
explicit $orderby now wins over the hardcoded by-name sort while the default is
preserved when absent.
3 more tests; the 293 existing server/fleet/agent tests still pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
on_restore_domain deliberately does not re-issue a certificate -- ssl_enabled
can still be True over cert files that expired while the row sat in the bin,
and quietly talking to Let's Encrypt is not something a "put it back" button
should do. That decision stays.
What was missing is telling anyone. The domain returns on plain HTTP while its
row still reads ssl_enabled, so the first sign is a browser warning.
A restore now has three outcomes instead of two: the side effect FAILED
(warning, error toast), it succeeded but left something actionable (notice,
warning toast), or it just worked. An on_restore hook returns a string to raise
a notice; it rides on the item so it cannot be confused with the error slot.
The notice only fires when nothing is actually serving HTTPS for that name --
app_vhost_kwargs re-attaches a covering wildcard, and warning about a domain
that came back on HTTPS anyway would just be noise.
3 tests: notice present, notice suppressed under a covering wildcard, and no
notice for a domain that was never HTTPS.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Field selection did not exist anywhere in the backend (`fields=`/`select=`: 0
hits across 90+ API files) and neither did eager loading (joinedload/
selectinload/subqueryload: 0 matches). /apps sends ~2,319 B per row where the
list UI reads ~411, over an unbounded `query.all()` -- for an admin with no
workspace header that is literally SELECT * FROM applications.
Ports the author's conuco_v2 query layer as app/api/_query.py, with two
deliberate differences:
- ORM, not Core. conuco resolves fields through `table.c`; here they resolve
against the mapper's column set, and that set is the ALLOWLIST -- an unknown
field is a 400, never a getattr into arbitrary model attributes.
- $select is not only a column narrowing. That is where the win actually is:
Application.to_dict() costs ~3 extra queries per row, and two of them
(image_scans, image_update_checks) are lazy='dynamic', so the attribute IS a
Query and NO loader option can hoist them. Declining a field is the only way
not to pay for it. to_dict(fields=...) now skips the loads it was not asked
for, so $select=id,name costs zero extra queries per row instead of three.
/apps also eager-loads server/project/environment/linked_app -- but only when
the request actually asked for the names they resolve, since a joinedload for
a field nobody selected is a wider row for nothing.
Every parameter is optional and to_dict(fields=None) is the full legacy
payload, so adopting this changed no existing caller's response. Paging meta
appears only once a caller pages.
$view is deliberately not implemented: ServerKit's saved views are per-user
rows already resolved in the browser, so a second server-side copy of the same
concept would only be somewhere for the two to disagree.
19 new tests. The 2 failures in the wider run (test_fail2ban_jail_service,
test_url_swap) reproduce on a clean tree -- pre-existing, not from this change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two gaps found by re-reading the seams rather than the pages.
useViewLink was wired into useTableChrome but NOT into useGridConfig, so
/domains -- the page the whole grid was built for -- was the one list page that
could not produce or accept a view link. Both hooks share it now, so a DataGrid
page and a DataTable page emit the same URLs.
SavedView had no pre_restore. Deleting a view frees its name AND its slug (the
unique indexes are partial), so either can be taken by the time you press
Restore; clearing the tombstone would then violate the index and 500 at commit.
Same guard Domain already had.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Making Domain soft-deletable silently changed the meaning of every existing
`Domain.query`: before, it could only return live rows. An audit of all 16 call
sites found ten that were now wrong, several with side effects outside the
panel.
The ones that reached the outside world:
setup_reconcile_service.py:267 the only is_primary row after deleting the
primary IS a tombstone -- and it drives a WP-CLI search-replace across
the whole WordPress database. It would have repointed a live site at a
host nginx no longer serves.
setup_reconcile_service.py:83 DNS backfill enumerated deleted domains and
called ensure_a_record() -- creating a real record at Cloudflare/Route53
for a domain the user removed.
api/domains.py:367 (enable_ssl) a real ACME order for a host that is in no
server_name: the HTTP-01 challenge cannot be served, burning Let's
Encrypt failed-validation limits for the whole registered domain, and on
success installing a wrong-CN cert on the LIVE vhost.
doctor_service.py:267 permanent red "does not resolve" rows that no
action could clear, and the same set is the allowlist for one-click DNS
repair -- so Repair created a provider record for a deleted domain.
And the ones that broke the feature itself:
domain_attach_service.py:79 a tombstone counted as a name clash, so you
could not move a hostname to another app after removing it. Migration 083
made the unique index partial precisely to free the name; this re-imposed
the burn in application code.
domain_attach_service.py:71 attach found the tombstone, reported success,
created no live row, published nothing -- and then mutated a recycle-bin
record.
site_domain_service.py:600 same for give_subdomain: success, a URL, a DNS
record, and a site that was never published.
api/domains.py:532 a second vhost renderer (admin Regenerate)
writing tombstones back into server_name.
Application.domains still includes tombstones ON PURPOSE -- filtering it with a
primaryjoin would, with delete-orphan, make SQLAlchemy HARD-delete the row the
recycle bin exists to keep. `Application.live_domains` is the accessor for
"what this app actually serves", and to_dict() uses it so the /apps payload no
longer lists deleted domains.
Per-domain routes now 404 a tombstone instead of editing it.
Seven tests cover the leaks that had real consequences.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
create_domain and delete_domain each called NginxService.create_site directly,
passing no ssl_cert / ssl_key / micro_cache. create_site takes those as
arguments, so the regenerated vhost came out HTTP-only and cache-less: on a
site covered by a base wildcard cert, touching ANY domain silently knocked the
whole site — every other domain on it — back to plain HTTP, and dropped its
micro-cache directives. Invisible until someone loaded the site.
Both now publish through SiteDomainService.write_app_vhost via one local seam,
so add and remove cannot disagree about what the served config should be. That
also fixes two lesser asymmetries: the rewrite was gated on app_type == docker,
so php/static/flask/wordpress vhosts kept advertising a domain after it was
removed; and it never called enable_site.
ALSO FIXES A REGRESSION I INTRODUCED WITH SOFT DELETE. app_vhost_kwargs read
`Domain.query`, which before soft delete could only return live rows and now
returns tombstones too. The vhost renderer would have written a deleted domain
straight back into server_name — the domain would keep being served after
deletion, and drift detection (which renders through this same function) would
report correct config as drifted. Now query_active().
Three tests, each failing against the previous code: the wildcard cert and
micro-cache survive in the resolved kwargs, a soft-deleted domain is absent
from them, and an end-to-end delete leaves the surviving domain on HTTPS.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The teardown map for delete_domain landed after the hook shipped and found a
real hole in it.
When the deleted domain was the app's LAST one, delete_domain runs
disable_site() + delete_site() -- the sites-enabled SYMLINK is gone, not just
the server_name. The hook called NginxService.create_site and reloaded, which
rewrites the file and leaves it UNSERVED. Restoring the last domain of an app
looked like it worked and served nothing.
SiteDomainService.write_app_vhost is the correct seam and fixes three things at
once:
1. it calls enable_site, so the symlink comes back;
2. app_vhost_kwargs re-attaches a covering wildcard cert and the micro-cache
flag -- a bare create_site drops both, so a restored site would silently
fall back to plain HTTP;
3. it handles every host-nginx app type. delete_domain only touches nginx for
docker apps, so php/static/flask/wordpress vhosts still list the deleted
domain; the rewrite is idempotent, so running it there repairs the
asymmetry rather than preserving it.
New test pins the case that was broken -- restoring an app's only domain must
call BOTH create_site and enable_site -- and it fails against the previous
implementation.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deleting a domain did two things: tombstone the row and rewrite the app's
nginx vhost so traffic stopped. Restoring only undid the first, so a domain
could be "restored" and still not served. on_restore_domain rewrites the vhost
from the app's live domains -- the mirror of what delete_domain does -- and
reloads.
Adds a `pre_restore` hook to the registry, because "restorable" is a moving
target: deleting a domain FREES its name (the unique index covers live rows
only), so something else may hold it by the time you press Restore. Clearing
the tombstone then violates the index and raises IntegrityError at commit -- a
500 for what is really an explainable conflict. Now it refuses with the name of
the app that took it. Same guard for a parent Application that no longer
exists.
A restored domain that was primary does NOT become a second primary: whatever
stayed live keeps the role and this row returns as an alias.
The certificate is deliberately not re-issued. ssl_enabled may still be true
with cert files that expired while the domain sat in the bin, and quietly
talking to Let's Encrypt is not something a "put it back" button should do --
the domain returns on HTTP and SSL is re-enabled explicitly.
Five tests, including the two that would otherwise be 500s: name taken again,
and app gone. A failed vhost write warns and KEEPS the restore rather than
rolling back a row the user asked for.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deleting is the only action in a control panel that repeating cannot undo, so
records a person destroys by hand now leave a tombstone and the Recycle Bin
hands them back.
SoftDeleteMixin adds deleted_at/deleted_by_id plus soft_delete/restore and
query_active()/query_deleted(). recycle_bin_service is a REGISTRY: a model
registers with a label function and an optional on_restore hook, so the API
lists and restores across types without knowing what a domain is. Adding the
next soft-deletable model is one register() call.
Applied to Domain (a real, destructive case -- deleting one used to lose the
record outright) and SavedView. DELETE /domains/<id> now tombstones and still
tears the vhost down; restoring a row does NOT re-do side effects unless the
type supplies on_restore, which is the honest default.
API: GET /recycle-bin (all kinds, newest first), POST
/recycle-bin/<kind>/<id>/restore, DELETE /recycle-bin/<kind>/<id> (purge,
admin-only -- the one irreversible action), POST /recycle-bin/purge-expired
with a 30-day default retention.
THE PART THAT NEEDED CARE: both tables had a plain UNIQUE covering what is now
a tombstone, so deleting a domain would have permanently burned its name and
deleting a view would have blocked re-creating one by the same name. Migration
083 replaces each with a PARTIAL unique index predicated on deleted_at IS NULL
-- supported by SQLite 3.8+ and Postgres. Tests pin exactly that: a deleted
name can be reused, and the live row stays unique.
Also gives SavedView a `slug` (backfilled), the handle the upcoming
?view=<slug> links resolve against. A tombstoned view does not reserve its
slug, so deleting a view frees the link name too.
Migration is idempotent on both columns and indexes: `deleted_at` is
index=True on the mixin, so a database built from the models already has that
index, and a bare create_index would abort the upgrade on exactly the installs
we most want to keep working.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The CPU/Memory/Disk gauges read `server.metrics`, the same key the
per-server status endpoint uses (servers.py:757). The list endpoint never
populated it -- to_dict() defaults to include_metrics=False, and even when
asked it emits `latest_metrics` -- so on a live panel every gauge cell fell
back to the "no data" dash.
It survived because the screenshot fixtures DO supply `metrics`
(sections.mjs:294): the UI looked correct in every capture and was empty in
production. The fixture had encoded the intended contract; the backend never
implemented it.
Batched deliberately: Server.metrics is lazy='dynamic', so the obvious
per-row lookup costs one extra SELECT per server. Newest row per server =
highest id (autoincrement on insert), which also avoids the tie a
max(timestamp) join hits for same-second rows -- two queries total,
regardless of server count.
Tests pin all three properties, and each fails without the fix: the key and
field names the table reads, absence (not {}) for a server that never
reported, and that the query count stays flat as servers are added.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The queue-bus consumers, job system and metrics collector share one SQLite
file with request handling, and the default journal mode lets any writer
lock the whole file — the sporadic 'database is locked' errors in the
notification/job consumers. Connections now open with journal_mode=WAL
(readers and a writer coexist), busy_timeout=30s and synchronous=NORMAL.
Skipped under TESTING, where conftest owns connection pragmas.
An authenticated user of any role could inject arguments into git
subprocesses running as the backend user (root under systemd):
- repo_url was passed unvalidated to git ls-remote/clone, enabling the
ext:: transport (host command execution on git < 2.52 defaults) and
file:// local-repository disclosure (all git versions).
- app_path on POST /deploy/clone was passed without a '--' terminator,
enabling --upload-pack=<cmd> option injection independent of git
version and protocol.*.allow policy.
- branch flowed into 'git fetch origin <branch>' as an unterminated
refspec.
Fix (new app/utils/git_security.py applied at every affected sink):
- repo_url scheme allowlist (https/http/git/ssh + scp-like SSH);
rejects ext::/file://, local paths, leading '-', whitespace.
- '-c protocol.ext.allow=never -c protocol.file.allow=never' pinned on
every remote-touching git call, plus a restrictive GIT_ALLOW_PROTOCOL
in the subprocess environment.
- '--' positional terminator before URL/path/refspec arguments —
co-load-bearing with protocol pinning, which alone does not stop
option injection such as --upload-pack=<cmd>.
- branch/ref validation (leading '-', ref-format metacharacters) for
clone --branch, fetch refspecs and stored deploy configs.
- POST /deploy/clone now confines app_path to the managed apps root,
mirroring how /apps/from-repository derives it server-side.
Adds 31 regression tests; bumps version to 1.7.86.
Reported-by: CaptBoykin
Co-authored-by: Tyler Boykin <25166954+CaptBoykin@users.noreply.github.com>
AuditService.get_logs gains a target_id filter (paired with target_type) and
/admin/activity/feed passes both through — the per-resource activity
timeline can now ask 'what happened to THIS app'.
SavedView model (user/page/name/state JSON/is_default, unique per
user+page+name), idempotent 082 migration, saved_view_service with
one-default-per-page enforcement, and a JWT-scoped /api/v1/views CRUD
blueprint. Covered by 9 endpoint tests.
With the remote index memoized, re-parsing every bundled YAML on each call
was the entire remaining cost of a Templates page load. Entries are now
memoized per file and keyed on (st_mtime_ns, st_size), so a sync or an edit
invalidates exactly the file that changed: measured on the real catalog,
list_local_templates() goes from 304ms to 0.89ms (118 templates), and the
full catalog listing from 1707ms to 1.26ms.
The writers need no invalidation call - mtime keying covers create, overwrite
and delete, the last because the listing only returns what the directory scan
finds. Cache entries are pruned to what is still on disk so a long-running
panel that churns templates cannot grow it without bound.
Failed parses are cached too, keyed on the same stat tuple, so one malformed
YAML no longer costs a parse on every render; it is retried the moment the
file changes. A file that fails to parse still does not claim its id, so a
valid file of the same name in the fallback directory can supply it - pinned
by a test, since that is easy to lose in a refactor.
Entries are deep-copied out: a shallow copy would share the nested
categories/repo/engine containers, so a caller appending to categories would
corrupt every later listing.
Also swaps os.listdir for os.scandir (DirEntry.stat() reuses data the scan
already returned) and sorts directory entries, which makes a same-id
collision (foo.yaml vs foo.yml) resolve identically on every platform instead
of depending on filesystem order.
Both bugs were latent only while DEFAULT_REPOS pointed at a 404. Now that
serverkit.ai proxies a live registry, they bite on every Templates page load.
list_all_templates() concatenated local + remote with no id merge, so every
bundled template was listed twice the moment a real registry answered.
Measured against the live registry: 224 entries for 116 unique ids. It now
merges by id with local winning, matching get_template(), which already
resolves local directories before any repo; among repos, the first enabled
one to claim an id keeps it.
fetch_remote_templates() ran inline on every listing with a 30s timeout and
no memoization, so one unreachable repo stalled the whole catalog on every
render. It is now memoized with a TTL (5m success, 60s failure), a 10s
timeout, and a last-good fallback so a transient blip cannot blank a catalog
the panel has already shown. The last-good payload is carried forward across
repeated failures rather than being overwritten by the first one. The cache
is dropped in save_config(), the single choke point for repo add/remove/
enable (sync_templates routes through it too), and force=True bypasses it.
11 proving tests, including the repeated-failure case and a regression test
that a cached entry cannot be corrupted by a caller mutating it.
DEFAULT_REPOS pointed at raw.githubusercontent.com/serverkit/templates --
the org `serverkit` does not exist (the registry is jhd3197/serverkit-templates,
and its default branch is master, not main). So it has 404'd for its entire
existence: no panel has ever fetched a template remotely, and every /templates
page has been showing only the 118 bundled backend/templates/*.yaml.
Now points at https://serverkit.ai/templates, which proxies the registry and
was built for this consumer -- it serves <repo_url>/index.json and the
<repo_url>/templates/<id>.yaml path this class derives, behind a TTL cache
with last-good fallback. Using the product domain also means a branch rename
upstream cannot silently empty every panel's catalog, which is the exact
failure this is fixing.
Fixing the default alone would only help fresh installs, because get_config
returns whatever a saved templates.json holds. Known-dead URLs are therefore
healed on read. Nothing is lost by rewriting a URL that never once resolved.
The checksum half is here rather than in a follow-up because this commit
ACTIVATES a download path that has never run: sync_templates wrote whatever
came back straight to disk, ignoring the sha256 the index pins for every one
of the 106 official entries. Turning on an unverified fetch days after the
panel learned to verify ed25519 signatures on extensions would be shipping a
known downgrade. Mismatch is now a hard refusal that never reaches disk;
absent hash is allowed and counted, mirroring unsigned-vs-invalid for
extensions. Content is written as bytes so the file on disk is exactly what
was hashed.
Note get_template's in-memory remote read (it parses without saving) is not
covered -- it has no index entry on hand and would need an extra fetch.
DEPLOY ORDER: serverkit.ai must ship the master-branch fix before this helps.
Until then the proxy 502s and sync finds nothing, same as today.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Runtime-ESM extensions (starting with serverkit-wordpress) need slot
mounting and error containment from the host; additive, versioned in
lockstep frontend/backend.
Create/update validation fails loudly naming the missing extension
instead of orphaning monitors; health sweep no-ops with zero WP sites
(plan 52 phase 3 task 17). StatusComponent.wordpress_site_id stays core
per D1.
The wordpress template kind + wordpress-external-db preflight move behind
a template-provider hook the extension supplies; the backend filters the
catalog so the WP card simply does not appear when WP is absent — no
frontend change needed (plan 52 phase 3 task 16).
Extensions register event types at load; wordpress.* types now come from
the WP extension instead of a hardcoded core catalog (plan 52 phase 3
task 15). emit_wp stays core as a dumb emitter helper.
VALID_TARGET_TYPES is now provider-driven; the wordpress_site branch
moves behind a provider the WP extension registers. ProtectionPanel /
RestoreDrawer read available types from the API; policies whose provider
is absent surface a clear missing-provider state instead of crashing
(plan 52 phase 3 task 13).
