5 Commits
Author SHA1 Message Date
Juan Denis 55d8fb1e44 Read route bodies through json_object()
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.
2026-10-10 20:53:43 -04:00
Juan Denis 97a96d963b Give common API errors stable codes and service wording
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.
2026-10-04 18:59:25 -04:00
Juan DenisandClaude Opus 5 031142111d fix(authz): give every mutating route an explicit authorization decision
GHSA-r4q7-x795-vr4j was one forgotten check, but the audit behind it found
the same shape across the surface: `@jwt_required()` authenticates, it does
not authorize, and ServerKit has no default-deny to catch the difference.
This closes the routes that audit left open — fleet-monitor alert
acknowledge/resolve, recycle-bin restore, nginx test/diff, registrar
sync/test, server-template drift checks, uptime tracking on/off, server ping
and remote compose ps/logs, refresh-capabilities, template
test-db-connection, deploy branches, buildpack detect — plus three inline
gaps: a viewer could widen their own API key's scopes, the demo-deploy flag
was doing double duty as an authorization one, and tagging a shared resource
had no write check at all while attaching one did.

`_ensure_group_mutable` only ever blocked *system* queue groups, so every
queue_bus mutation now also calls `_ensure_group_accessible` (owner-or-admin)
— renaming, draining or deleting another user's group was open to any
authenticated caller.

The rbac policy decorators authenticate through `auth_required()`, so they
replace `@jwt_required()` rather than stack under it; stacking rejects
API-key callers before the policy can authorize them
(tests/test_auth_decorator_boundaries.py enforces this). Where a route then
needed the caller it reads `get_current_user()` — `get_jwt_identity()` raises
for an API-key request. The decorators also carry a `_sk_authz` marker so a
static test can see the decision from the source.

`migrations.get_migration_history` moves to `require_admin_user()` for the
same reason its siblings did: the inline `User.ROLE_ADMIN` check it kept was
already reading two names this module no longer imports, so the route raised
NameError for every caller, admin included.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-19 21:44:17 -04:00
Juan Denis 8ca1f3d07e refactor(auth): enforce one policy boundary per route 2026-08-18 02:42:39 -04:00
Juan DenisandClaude Opus 5 f49a3e6cfc feat(recycle-bin): soft delete for records, with restore and purge
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>
2026-08-12 07:14:55 -04:00