The three restore-point read routes authenticated with bare
@jwt_required(), pushing the census to 579 against a 577 ceiling.
They now use the auth_required() policy door like the file's mutating
routes (which already accept API keys via @developer_required), keeping
read access consistent with write access for API-key callers; scope
authorization stays in-route. test_restore_point_env.py hand-rolled
Application/User seeds — switched to the make_application/make_user
factories. Migration inventory regenerated.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The curated serverkit-recipes index is consumed like the themes registry
(cached remote index, bundled fallback, SERVERKIT_RECIPES_REGISTRY_URL
override). GET /recipes/registry[/<slug>] browses it; POST /recipes/runs
now accepts a registry_slug plus server_id — targeting a server finds or
creates the per-workspace Recipes project — and validated non-secret
params that pre-supply handoff inputs so only true secrets pause the run.
Frontend: Recipes catalog page (cards, category chips, install drawer
with running-server picker, preflight table, up-front inputs and an
asked-during-the-run secrets preview) under new Services and Marketplace
tabs; runs land in the deployment console.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Expose disk_reclaim_service behind /api/v1/system/disk/reclaim: a
read-only report and a confirm-gated POST whose keys are re-validated
against a fresh scan server-side (safe candidates only — telemetry
stays CLI-only). ?wait=false runs it as a background job; both paths
are audit-logged.
The curated fix shows up where the problem is reported: a failing
disk.headroom doctor check and an alerting disk tile each offer
'Reclaim space', which previews measured candidates before handing
the run to Operations.
The attachment round added two hand-shaped `jsonify({'error': str(exc)}), 400`
returns in api/ai.py, which pushed the error-shape census to 1146 against a
ceiling of 1144 and turned shard 2 red. The census policy is not "raise the
ceiling" -- new endpoints raise, and these are new.
AttachmentValidationError now subclasses app.exceptions.ValidationError, so the
global handler owns the 400 body, the `code`, and the X-Request-ID correlation.
It is still a ValueError (ValidationError subclasses one), so every existing
`except ValueError` caller and the pytest.raises assertions in
test_ai_attachments.py are unchanged, and the two try/except blocks in the
routes disappear entirely.
Census back to 1144 = ceiling. Verified: test_ai_attachments, test_ai_assistant,
test_error_shape_ratchet all green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- LocaleContext: reset ready on each language change so non-default
locale bundles trigger a rerender once active
- auth PUT /me: raise typed ValidationError (auth.unsupported_language)
instead of a hand-shaped error body; error-shape ceiling 1145 -> 1144
- i18n.py docstring: point at the real parity test
(test_user_language_preference.py)
SDK 1.2.0 -> 1.3.0 (additive).
Auditing the extension surface for plan 79 found the localization gap I
expected and two pre-existing bugs I did not.
The gap: extensions had no way to translate anything. The SDK now re-exports
the HOST's translation surface -- useTranslation, Trans, t, useLocale,
useLabel, useFormat and the intl door -- and i18next/react-i18next join the
shared vendor singletons. That sharing is the load-bearing part: an extension
bundling its own i18next would get a separate, uninitialised instance, so
every t() in it would render its English default forever and nothing would
report it. Exactly the "two React copies" hazard vendorShare already documents,
and exactly the failure mode this plan hit twice in core. They also join
dedupe/optimizeDeps for the same reason React is there.
The bugs, both caused by the same duplication:
- The import map listed `react` and `react-router-dom`, but no shim existed
for either. A runtime extension importing bare `react` 404'd at load.
- The serverkit-sdk shim re-exported 26 of the SDK's 71 exports. An extension
importing Button, Modal, EmptyState, formatBytes or timeAgo from the SDK got
`undefined`. The shim's own comment claimed parity was "asserted by the
frontend build + backend test_sdk_contract"; nothing asserted it.
Three files independently restated the same module list -- the import map, the
namespace capture, and one hand-written shim per module -- so they drifted.
Now vendorManifest.js is the list, generate-vendor-shims.mjs derives every
shim's exports (from the real package's namespace, or from the SDK entry's own
export statements -- never typed by hand), the import map is built from the
same manifest, and `--check` fails the build on drift. 26 -> 85 exports on the
SDK shim, 6 -> 8 shims, all present.
Manifest contributions gain an optional `labelKey` beside `label` for nav, tabs
and command-palette entries, resolved at render by the same translateLabel core
uses -- data cannot call t(), and a label resolved at import would translate
once and never follow a switch. Omitting it renders `label` verbatim, which is
what every existing extension does, so nothing breaks.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 79 §E wave 2 + F1.
Login, Register and Setup go to zero untranslated literals. These screens run
before anyone is signed in, so they are what proves the precedence chain: with
no user row to read, the locale comes from localStorage, then the panel
default from /auth/setup-status, then the browser.
F1 turned out smaller than the plan assumed, and in a good way. The plan
proposed inventing an error-code contract; the repo already has one -- plan 76
milestone B's typed-error door emits { error, status, code, request_id }. So
the work was to USE it: fourteen auth returns become typed raises carrying
stable codes, and the API client maps the code to a translation key with the
server's own English as the default. The 257 `toast.error(err.message)` sites
keep working untouched and gain translations as codes land. Error-shape
ratchet 1159 -> 1145.
The first cut mapped `t('errors.' + code)`, which is wrong in a way worth
recording: a computed key is invisible to the extractor, so those strings
would never reach en.json and a translator would have no way to learn they
exist. The registry is an explicit literal table instead -- it doubles as the
reviewable list of which server errors are actually translated, and anything
absent falls through to English, which is today's behaviour.
Only auth carries codes so far. Plan 76 §C measured 23 sites that choose an
HTTP status by grepping their own error prose; translating that prose would
turn those 403s into 400s, so the rest waits on milestone C rather than
shipping a silent status regression.
test_login_links asserted the bare {'error': ...} body by equality. Updated to
assert the typed contract including `code` -- asserted rather than tolerated,
since dropping the code would silently make the message untranslatable again.
Ceiling 8087 -> 8055.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 79 A1/A2/A4 + B1/B2.
Delivery deliberately avoids the usual http-backend fetch. `en` is bundled;
other locales are dynamic imports Vite emits as chunks from the app's own
static origin. The sign-in and setup screens render before any API call
succeeds, so a fetched locale either flashes English or -- when the backend
is the thing that is broken -- never arrives, on the one screen whose job is
to say so. It also keeps air-gapped and subpath installs working with no
second path to 404.
Precedence (user row -> localStorage -> panel default -> navigator -> en)
mirrors ThemeContext's, including the split provider: LocaleProvider sits
outermost so a toast raised during theme init is already translatable, and
LocaleSync bridges auth state down, exactly as ThemeSync does.
users.language is nullable on purpose -- "follow the panel default" is a
different state from an explicit choice of English, so existing rows are not
backfilled. The backend restates the supported list to validate writes; a
parity test fails the build if it drifts from the frontend manifest, which
stays canonical.
Two bugs the tests caught, both real rather than test artifacts:
- normalize_language('ES') returned None. The shape gate was case-sensitive,
so two of the three forms browsers actually send were rejected.
- PUT /me is throttled 3/minute for credential edits, and language rides the
same route. Switching language three times returned 429 -- and because the
client applies the change locally first, the preference silently failed to
persist and reverted on the next sign-in elsewhere, defeating the column.
Cosmetic-only payloads are now exempt from that limit; credential edits
stay throttled (proved both ways) and the app-wide 100/minute still holds.
eslint moves to ecmaVersion 'latest' because import attributes are ES2025 and
Node's test runner requires them for the manifest imports.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
POST /api/v1/workspaces/ was gated only by @jwt_required(), so any
authenticated user - including the read-only viewer role - could create
a workspace, automatically become its owner, then add arbitrary members
and mint workspace-scoped API keys (GHSA-r4q7-x795-vr4j, CWE-862).
Add @admin_required, matching the owner/admin checks every other
mutating workspace route already enforces, plus a regression test
asserting viewer and developer JWTs get 403 while an admin still
creates.
Co-authored-by: Alex <134992700+campguyuz@users.noreply.github.com>
Twenty-one imports whose last caller left during the plan-76/77 sweeps:
mostly 'import subprocess' in services now routed through the shared
runners, plus time/requests in the CachedRemoteIndex adopters, threading
in agent_gateway, os in api/system, User in api/bandwidth, run_privileged
in nginx_advanced_service, and the dead SENSITIVE_SETTING_PARTS alias in
audit_service (zero importers). Each removal AST-verified unused before
deletion; all modules compile and the app factory boots.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The full-suite run tripped four ratchets on plan-77's own additions —
exactly their job: /api/v1/runs moves onto auth_required (the JWT-only
ceiling holds at 605) and behind run_log_service.list_run_logs (no
model access in the controller); the core-blueprint snapshot records
runs_bp deliberately (108 -> 109 + sha); sso_service joins the crypto
allowlist (C2 dual-read lives there by design); the controller-boundary
baseline re-records the two pre-existing ManagedDatabase crossings under
their B5 query_active spelling (count unchanged).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
First of the regret-ranked adoptions: a plain delete now tombstones the
row (migration 088; no unique constraints, so no partial-index rewrite).
The database content and its managed BackupPolicy survive the bin window
— restore brings everything back intact; PURGE removes the policy but
NEVER drops database content; the explicit ?drop=true path stays
immediately destructive. Active read paths (list/get/adopt/manifest/env
references/app detail) switch to query_active(); the backup executor
deliberately stays tombstone-blind so bin-parked data keeps backing up.
Server (98 query sites, fleet enrollment + HMAC secrets) is the next
ranked adoption — deliberately its own pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RunLogStream accepts any RunLike (__run_kind__ + id + to_dict, optional
result/step members): one room grammar run_<kind>_<id>, one batched
persisted run_log/run_status event pair, and a GET
/api/v1/runs/<kind>/<id>/logs?after_id polling twin over the new
run_log_entries table (migration 087). Deploys keep DeploymentJobLog and
dual-emit the legacy deploy_log/deploy_status during the migration
window. The unified Job runtime — which had NO live channel — adopts it
in the consumer (start/succeeded/failed frames; handlers can log via
stream_for(job)). The dead pipeline_* scaffolding was retired in E2.
Gate: emit -> persisted row -> after_id resync contract test; consumer
adoption test. MIGRATION_INVENTORY.md regenerated (census moved).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
config.py's parsers move to app/utils/env.py and are imported back; the
three incompatible truthiness variants collapse to one TRUTHY tuple.
Fixes the measured bug: SERVERKIT_ALLOW_PRIVATE_DOWNLOADS=on silently
read false in plugin_service (its tuple lacked 'on'); same for the pip
gate, SERVERKIT_STAGING, and SERVERKIT_DEMO_MODE. Ratchet: no new
os.environ truthiness tuples outside utils/env.py.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
agent_registry.authenticate_and_register owns the full handshake
(rate-limit -> HMAC -> allowed-IPs/anomaly -> new-IP -> version ->
register) and ingest_agent_state owns heartbeat/sysinfo/capability
side-effects; the WS namespace and long-poll endpoints become thin
adapters translating error codes to emit/disconnect vs jsonify/status.
The per-IP auth limiter moves to the registry (re-exported from the
gateway for existing callers/tests). Gate: parametrized ws/poll matrix
(bad HMAC, blocked IP, rate limit, new-IP anomaly) + a word-anchored
ratchet that the adapters never re-grow the sequence.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Six more models own their *_encrypted columns through the descriptor
(linked_panel, source_connection, container_registry, managed_database,
registrar_connection, pending_agent); eight service/api files stop
importing utils.crypto for them. legacy_plaintext mode mirrors
decrypt_secret_safe so pre-encryption rows keep reading until rewritten
— this also fixes registrar/source-connection reads that used the
RAISING decrypt variant on columns that may hold legacy plaintext.
Ratchet test freezes the remaining service-level crypto importers.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QueryParseError is now a ValidationError subclass — the global handler
answers it as a 400 and per-route translations become unnecessary (the
existing ones keep working). PageQuery parses, defaults, and bounds
page/per_page in one place; admin audit-logs, activity feed, error logs,
and telemetry events adopt it (telemetry keeps its 200 cap). git.py and
source_connections.py keep hand-parsed params deliberately — they proxy
pagination to Gitea/GitHub/GitLab/Bitbucket, not to DB queries.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The inbound PR webhook must keep answering 200 (providers disable failing
hooks), but its preview-settings crash only reached logger.debug — the
last route swallowing an exception without putting it on the record. It
now goes through record_unexpected before the in-band answer.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DbConfigTunerService raises ValidationError (preconditions/settings),
DependencyUnavailableError (engine/container operations, with the
restore-then-report semantics preserved in the message), and NotFoundError
(nothing to roll back to); db_tuner.py drops its three error-key
ternaries. The census docstring now records the completed migration and
corrects its own stale 'eight callers' blocker. Census 3 -> 0 — held as an
invariant from here.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SecretVaultService, SecretService, and WebhookGatewayService raise typed
errors and return bare data; secrets_webhooks.py drops all twelve
{'success': bool} translations including its two prose sniffs. Bulk
create keeps per-item error collection (catching ApplicationError).
Replay now answers 200 with success:false when the forward target refuses
— previously a failed forward was a 404 with error:null — and the
WebhooksTab toast distinguishes the two. The public /webhooks/receive
wire protocol is intentionally unchanged (external senders depend on it).
Sniffing census 6 -> 4.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
theme_registry_service.install returns the theme dict and raises
NotFound/Validation/DependencyUnavailable; the route's not-found prose
sniff is gone. import_theme keeps its tuple contract for the import
route. Sniffing census 7 -> 6.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
update_ip raises AuthenticationError for a bad/disabled token (401 via the
global handler, as before) and ValidationError for a bad IP; the route's
except-ValueError message sniff is gone. Sniffing census 8 -> 7.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
enqueue_scan/resolve_app_path raise NotFoundError/ValidationError; the
scan routes drop their except-ValueError prose sniffs and let the global
handler map. Sniffing census 9 -> 8.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
check_application raises NotFoundError/ValidationError and returns the
check dict bare; the route stops re-deriving status codes from message
text. Sniffing census 10 -> 9.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
saved_view_service raises ValidationError/NotFoundError/ConflictError and
returns bare dicts; views.py drops the (value, err) tuple translations and
its not-found prose sniff. Duplicate names are now 409. Sniffing census
11 -> 10.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The C1/C2 reference conversion: FileService raises typed errors
(PermissionDenied/NotFound/Conflict/Validation) and returns bare data;
files.py stops translating {'success': bool} envelopes and stops choosing
status codes by sniffing error prose. Denials are now 403 regardless of
wording, missing paths 404, already-exists 409. Success bodies keep the
compatibility 'success': True key so consumers are unchanged. The plan's
'eight callers' blocker was a substring-grep artifact — FileService has
exactly one consumer. Sniffing census 23 -> 11.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Migrate all 20 direct agent_registry.send_command call sites (api/servers,
terminal, fleet doctor/repair/fleet incl. the upgrade Thread target,
deployment_runner, survey, tunnel broker/publish, plugins SDK) onto the
shared dispatcher, and widen the ratchet from remote_*_service.py to a
tree-wide AST scan covering builtin-extensions/. The survey test's
transport stub gets the transport's real keyword signature.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Follow-up to a7d5a0b9, and a hole in its ratchet.
queue_bus.py answered 21 crashes through its own `_handle_error(e)`, which
returned 500 with str(e) for anything that was not a QueueBusError. Those are
exactly the unreported crashes milestone B set out to remove, and the census
missed every one of them: it looked for a literal 500 in the except handler, and
here the 500 lived one call away in the helper.
A ratchet that a one-line helper defeats is not a ratchet, so the census now
follows one level of indirection — a return of a same-module function that
itself answers 500 without reporting counts as answering 500. Verified
non-vacuous with a probe module using exactly that shape.
`_handle_error` now keeps a QueueBusError's own message and status (it is an
expected, caller-actionable failure) and routes anything else through
unexpected_response, fixing all 21 sites at once.
Worth recording how this was found: measuring the "handlers that swallow and
answer 200" population, which the plan had at 56. The real number is 1 — the
earlier figure counted any handler whose return the AST could not resolve to a
status, which included every `return unexpected_response(exc)` the migration had
just introduced and every `return _handle_error(e)`. The plan is corrected.
Gate: -k queue_bus (66 passed), crash ratchet + reporting tests (20 passed).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 76 milestone A, the third row. 23 handlers decided admin access inline,
all in two files, in two shapes.
18 were the mid-route gate this milestone already built a door for:
current_user_id = get_jwt_identity()
user = User.query.get(current_user_id)
if user.role != 'admin':
return jsonify({'error': 'Admin access required'}), 403
which is four lines to say require_admin_user(), and carried the two-line
identity lookup with it. Migrating them closes both rows at once: the identity
census drops 87 -> 69 and the controller-boundary baseline loses 18 persistence
crossings (528 -> 510), with no new entries.
It also fixes a latent 500: where the check omitted the `not user or` guard, a
token naming a deleted user hit `None.role` instead of answering 403.
The other 5 are `not user.is_admin and app.user_id != user.id` — admin OR
owner, which is a different question. Those keep their shape and only converge
the truth source from `role != 'admin'` to `is_admin`; routing them through
can_access_app would silently widen access, since that also honours resource
grants.
The migration initially left `current_user_id` referenced but unbound in four
places. Two were caught by tests; the other two (create_manual_app,
upload_app_archive) have no coverage and were caught by an AST scan for names
used-but-never-assigned. Worth remembering that a mechanical removal needs the
scan, not just the suite.
Gate: -k "system or apps or app_ or host_snapshot or manual_app or upload",
304 passed / 13 skipped; all seven ratchets green (52 passed).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 76 milestone B, drift/bug row 4. 43 API handlers answered HTTP 500 from
their own `except Exception`, so the global 500 handler never ran — and that
handler was the ONLY caller of error_log_service.record_error(source='backend').
Every one of those crashes was absent from /monitoring/errors, and the page read
empty precisely because someone had "handled" them.
35 of the 43 also returned str(exc) to the caller. Two returned the full
traceback (templates.py, which also print()ed it), and several logged with a
locally-minted uuid4 'ref' that correlated with nothing but a log line.
The recording side of the 500 handler moves to app/error_reporting.py, so it is
importable rather than reachable only by not-catching:
record_unexpected(exc) -> logs, rolls the session back, records, returns
the request id
unexpected_response(exc) -> the above plus the documented crash body
The 500 handler now calls the same pair, so there is one recorder rather than
one recorder and 43 routes that quietly opted out of it.
Caller-visible change on those endpoints: the body is now the documented
{'error': 'Internal server error', 'status': 500, 'code': 'internal_error',
'request_id': ...} instead of the exception text. That is the point — an
unexpected exception's text is written for whoever reads the log and routinely
carries connection strings, absolute paths, and SQL. The request id is the part
a caller can act on, and it matches the row in the error log. No frontend code
read the 'ref' or 'trace' keys this drops (checked).
Two sites keep their own wording and call record_unexpected instead: sso.py's
login failures should not read as a generic crash on the login page.
docker.py's bulk endpoint keeps its per-item `results` — the caller's half-done
work survives, only the exception text is dropped.
Ratchet: tests/unreported_crash_census.py + its test, held at **0**, not a
legacy baseline to shrink. It counts blanket handlers that answer 500 without
calling either reporter, and is scoped to app/api — a service catching an
exception and returning a value is making a domain decision, not declaring an
HTTP crash. Verified non-vacuous: an injected handler fails it, and it passes
again on revert.
Measured and deliberately still open: 56 API handlers swallow a crash and
answer 200. A failure reported as success is worse, but converging it is
milestone C's response-envelope work, not this row.
Gate: tests/test_unexpected_error_reporting.py (11, including the two that pin
why the old shape lost the crash), test_unreported_crash_ratchet.py (9), and
the 66 test files covering every touched module: 1105 passed, 7 skipped, 1
pre-existing Windows-only failure (test_fleet_doctor's socket.gaierror).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fe02ce8e removed queue_bus.py's private _current_user(), which was the only
code using the model. The automatic dead-import sweep kept it because the word
"User" also appears in a docstring two lines into _ensure_group_accessible.
Gate: -k queue_bus, 26 passed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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 76 milestone A, drift/bug row 3. Five API modules had grown a private
_require_admin() with *three mutually incompatible* return contracts:
jobs.py -> returns the user; None means DENIED
dns_cutover.py -> returns None on success; a response tuple means denied
waf.py -> same as dns_cutover
manifests.py -> (user, None) on success, (None, response) on denial
themes.py -> same as manifests
jobs.py's is the exact inverse of dns_cutover.py's, so a guard copied between
the two reads identically at the call site and fails open. Nothing in the code
told you which contract you were looking at.
Replace all five with rbac.require_admin_user(), which raises a typed
PermissionDeniedError the global ApplicationError handler already renders. A
gate that raises has no return contract to get backwards.
Two convergences ride along, both toward the stricter shared behaviour:
- manifests.py gated on `user.role != 'admin'` where the others used
`is_admin`. These are the same predicate today (is_admin is defined as
role == ROLE_ADMIN), so no route changes status, but there is now one
truth source rather than two.
- none of the five checked is_active, so a deactivated admin still passed
them; @admin_required has always checked it. The shared door checks it, so
these 25 call sites now reject deactivated admins like every other
admin route.
The unknown-user case keeps its 403 'Admin access required' rather than
becoming a 401/404: a non-admin caller should not learn from the status code
whether the account behind the token still exists.
Gate: tests/test_jobs.py, test_themes.py, test_waf.py, test_dns_cutover_api.py
(82 passed) and -k manifest (152 passed).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Plan 75 §G1, the remaining 33 files. Every raw subprocess call site in
backend/app that can go through a door now does. What is left is what
should be:
8 app/utils/system.py the doors themselves
8 database_service mysqldump|gzip, gunzip|mysql, pg_dump|gzip,
gunzip|psql — a pipe is two processes
7 build/docker/db_sync/ftp streaming output, gzip pipes
1 log_service tail -f
2 serverkit-analytics sibling repo; see below
Extensions were migrated in BOTH trees. builtin-extensions/ is the source
and backend/app/plugins/ the live copy, compared by normalised hash;
test_builtin_extension_drift stays green. serverkit-email has both sides
tracked, serverkit-ftp and serverkit-cloud-provision only the source.
serverkit-analytics (2 calls) is deliberately untouched: it has no source
in builtin-extensions/ at all, and backend/app/plugins/serverkit-analytics
is a gitignored materialised copy. Editing it would be a change that
cannot be committed and is overwritten on the next sync — that fix belongs
in the serverkit-analytics repo.
Exit-code semantics are preserved wherever they carry meaning, with a
comment at each site, because they are the thing a mechanical migration
destroys: clamscan 1 = INFECTED, grype 1 = vulnerabilities FOUND, yara
1 = matches, cron_runner 127 = never launched. Those read
result['returncode'], never result['success'].
run_checked gained two things real call sites needed: it yields when the
caller directs the streams itself (writing straight to a log file), rather
than adding capture_output on top and raising ValueError; and text/errors
pass through, so site_importers/ssh keeps decoding with errors='replace'
instead of gaining a UnicodeDecodeError on a bad byte.
A regression this migration introduced, caught by the suite and fixed:
install_profile_service._docker_usable() is deliberately TRI-STATE —
None means the probe could not run, because inside LXC the docker client
is usually present while the daemon never started, and callers "must not
treat a broken probe as proof of absence". Writing it as
run_checked(...)['success'] turned an exec failure into False, i.e. "docker
is not usable" — a fact nobody established. That is the §A violation this
plan exists to remove, reintroduced by the migration meant to prevent it.
test_probe_honesty_doctor caught it. Every other run_checked(...)['success']
caller was then audited; the rest are genuinely two-state, and their
originals also returned False on a missing binary.
Seven more slips of the same shape were caught before running by an AST
check for `x = run_checked(...)` followed by `x.stdout/.stderr/.returncode`
— a dict read as an object, which raises only on the failure path, so
happy-path tests stay green.
api_controller_boundary_baseline.json: pure deletion. Routing api/docker.py
through DockerService removed the last 6 subprocess crossings, so the
`subprocess` category is gone from the API layer entirely.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
