328 more routes used a JSON body as a dict straight from
request.get_json(), so a body of null, [] or a bare string crashed them
into a 500. Bare reads become json_object(); the
'get_json(silent=True) or {}' reads of action routes become
json_object(optional=True), which still treats a missing body as {}.
Routes that take arrays or pass the body through untouched are left
alone. The full backend suite shows no new failures.
Also carries three files' bare section citations (apps, backups,
notifications) and the extended citation guard, which needed them.
Nine route files each loaded the live app and checked access, with
four return shapes, docstrings promising error tuples the code never
returned, and 41 dead 'if err: return err' branches. require_app()
backed by ResourceGrantService.get_app_for does it once; conceal=True
keeps the 404-for-denied answer the previews, WAF and slot routes
already gave, so no route's status changes. Nine query sites leave the
API layer.
Comments, docstrings and a few strings cited plan numbers ("plan 29
#9", "plan 86 §B2") from a git-ignored folder, so they pointed at
nothing; one reached users in a bottleneck hint. Code and frontend
strings are unchanged apart from those citations (checked by AST and
token comparison). A vhost still carrying the old asset comment reads
as undrifted, and a guard test keeps new citations out.
Toasts showed backend prose verbatim, with no code to translate
against and "application" where the UI says "service". Not-found,
missing-field, permission, conflict and agent-offline errors now come
from factories in app/exceptions.py that pair one fixed message with
a dotted code (not_found.<resource>, validation.required,
permission.denied, conflict.exists, agent.offline), so the client can
translate them.
The shared app-loading helpers and the apps, domains, servers,
databases and backups routes raise those errors instead of hand
shaping bodies, which drops the hand-shaped body count from 1132 to
850. Five routes picked an HTTP status by comparing error text; they
now key off a result code or a sentinel so rewording can't change a
status. Five broad excepts that echoed str(exc) now log the crash and
return the request id instead.
Plan 76 milestone A, drift/bug row 2 — scheduled as a fix, not a refactor,
because it is a behaviour difference shipping today.
rbac.get_current_user() is the only lookup that knows about both auth
mechanisms: it returns g.api_key_user for an X-API-Key request and falls back
to the JWT. 73 call sites answered "who is calling" with
`User.query.get(get_jwt_identity())` instead, behind 19 near-identical private
helpers (12 named get_current_user(), shadowing the real door).
The audit that scheduled this row predicted those return None for API-key
callers. Measured, it is worse: auth_required() deliberately skips
verify_jwt_in_request() once the key middleware has authenticated, so there is
no JWT context and get_jwt_identity() *raises*. Nine handlers reached through
an API-key-capable policy decorator therefore returned 500 to every API-key
caller:
bandwidth.get_app_bandwidth @viewer_required
servers.set_server_workspace @developer_required
servers.create_server @developer_required
sso.get_sso_config / update_provider_config / update_general_settings
source_connections.update_{github,gitlab,bitbucket}_admin_config
source_connections.complete_github_app_manifest
databases.execute_{mysql,pg,sqlite,docker}_query (see below)
Three service helpers had the same lookup inside a blanket `except Exception`,
which converted the raise into a silent user_id=None — the write landed and
the audit trail lost the actor (shared_resource_service, cf_ops_change_service,
secrets_webhooks).
The four databases.execute_*_query sites were a second admin check *inside* an
already-@admin_required route: dead code that always passed for JWT callers and
crashed for API-key ones. They now call require_admin_user() rather than
re-deriving the answer.
Also folds in a sixth _require_admin() found in the cloudflare-ops extension
source (29 gates, the same inverted contract as jobs.py), and fixes
serverkit-gui's blueprint at its sibling-repo source rather than in this repo's
live copy.
Ratchet (second-wave rule 1 — a migration without one regrows):
tests/identity_door_census.py counts both bypass shapes by AST, direct and the
two-line `uid = get_jwt_identity(); User.query.get(uid)` variant. The ceiling is
87, all of them on @jwt_required()-only routes where reading the JWT is correct
today; the ratchet's third assertion holds the API-key-capable population at
exactly zero, so migrating one of those routes to a policy decorator without
fixing its lookup now fails. Verified non-vacuous: an injected bypass fails both
assertions and passes again on revert.
The first wave's controller-boundary baseline is regenerated — this removes 74
persistence crossings (593 -> 519), a debt reduction with no new entries.
Gate: tests/test_api_key_identity.py (7, each red before the fix),
test_identity_door_ratchet.py (10), test_api_controller_boundaries.py (4), plus
the 65 test files covering every touched module: 1024 passed, 10 skipped.
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>
`GET /databases/engines/<app_id>/extensions` carried only `@jwt_required()`, so
any signed-in user could enumerate any instance on the panel — the image it
runs, its databases, and what it can load. All facts about someone else's
application.
It gates on `can_access_app` now, mirroring the per-app docker DB read next to
it: owner, admin, or an explicit grant. Read access rather than admin-only is
deliberate — a developer who can already see an app should be able to see what
that app can load. The POST was already admin-gated and is unchanged.
The file also declares its `# Bucket: PER-APP` header (plan 29 #9), which is
what the structural sweeps read to know a per-app route file has been thought
about at all.
Found by the repo's own authz sweeps in a full-suite run — four failures, one
cause. Worth noting they fired on a route that has no feature test capable of
catching this: every functional test authenticates as someone entitled to the
data. That is precisely what a structural sweep is for.
Four tests pin the behaviour: denied without access, still visible to the owner,
404 rather than 403 for an app that does not exist (conflating them would make
the endpoint useless to its own caller), and 401 anonymous.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The third deferred item. An extension installs INTO a running engine rather than
getting its own container, so it lives in its own namespace:
`backend/templates/extensions/<id>.yaml` with a top-level `extension:` block,
discovered by the same operator-wins two-directory scan the app templates use.
Dropping a YAML in makes it available with no code change, and a test pins that.
Not nested inside the engine template, because pgvector applies to any
PostgreSQL-speaking engine rather than to `postgresql.yaml` — nesting would force
duplication per host. Not a third `kind:` in the app catalog either: an
extension has no compose, port or volume and produces no Application, so every
consumer of `list_all_templates` would have had to learn to skip it.
The hard part is not the SQL. `CREATE EXTENSION vector` is valid and fails on
stock `postgres:16` with "could not open extension control file", because that
image does not ship the library. Two layers, and the statement is never sent
speculatively: an extension declares which images provide it, so a listing marks
an instance unsupported BEFORE anything is clicked; then at install
`pg_available_extensions` is queried first and a negative answer is a 409 with a
remedy, not an attempt. The live probe overrules the image name in both
directions — a private `registry.internal/ops/pg-with-vector` is allowed, and a
`pgvector/pgvector` tag that does not actually carry the library is still
refused.
ServerKit does not recreate a container behind the operator's back. The remedy
instead names an engine template that IS capable, found by matching the
extension's images against each engine template's compose — hence
`postgresql-pgvector.yaml`. TimescaleDB deliberately ships no bundled host and
says so rather than implying one exists.
Compatibility also needed correcting mid-build: matching on wire protocol alone
claimed CockroachDB could host pgvector. It answers on pgwire and cannot load a
single extension. Engines opt in with `engine.extensions: true`, which excludes
it without any Python naming it.
Two fixes fell out. `DatabaseService` had no psql twin of `docker_mysql_execute`
— `managed_db_user_service` carried a private copy with a comment saying as
much; there is now one implementation and it delegates. And that twin sets
`ON_ERROR_STOP=1`, without which psql reports SUCCESS for a statement the server
rejected.
57 tests, covering the zero-code-change property, compatibility, the image
problem in both probe directions, admin gating, and refusals that must not
execute anything.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
You could not install a database from ServerKit. The tree's four roots were
literals, only MySQL and PostgreSQL reported real state, and no install path
existed anywhere in the backend.
Rather than add one, this makes a database engine an ordinary app template that
declares a top-level `engine:` block — family, wire protocol, admin user, which
variable holds the password, what it calls a "table". Presence of that block is
the only thing that lists a template in the Databases catalog, and installing it
is the template pipeline that already existed: same deployment job, same
streaming console.
The consequence is the point. Adding CockroachDB is adding a YAML file. So is
adding something we have never heard of — author it locally or sync a template
repo — and the install form still works, because it is generated from the
template's own `variables[]` rather than hand-written per engine. There is no
engine model, no engine table, no engine install mechanism to keep in step.
`test_dropping_a_new_engine_yaml_lists_it` is the design; if it ever needs code
to pass, this went wrong.
Twelve engines ship: postgresql, mysql, mariadb, mongodb, redis, valkey,
cockroachdb, clickhouse, influxdb, neo4j, meilisearch, couchdb. Each pins its
image, binds to loopback, names a data volume, and was checked against the
upstream image's actual environment contract — a wrong variable name is a
container that boots with authentication silently disabled.
Two safety notes worth reading. Engines bind 127.0.0.1 unless explicitly
exposed, and the flag is applied after caller variables so a handcrafted payload
cannot quietly publish one. And CockroachDB single-node runs `--insecure`
because password auth needs a certificate-backed cluster: for engines with no
credentials at all the loopback bind is the only control, so public exposure is
refused outright rather than warned about. A test pins the passwordless list to
exactly cockroachdb, so a future template that drops its password variable
fails loudly instead of opening a hole.
Also surfaced at last: `ManagedDatabase.owner_application_id` has been accepted
by the create endpoints for ages with no UI to set it. Creating a database now
offers "Attach to application".
Deferred deliberately: new-table builder, SQL dump import, engine extensions
(pgvector, TimescaleDB) — each a data-mutation surface deserving its own round.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Juan Denis