The comment explaining why scripts/ is copied into the image read
"/api/v1/servers/install.sh and /install.ps1", which reads as though the Windows
installer is served at the domain root. It is not — both live under
/api/v1/servers/. A comment whose whole job is to stop the next person hunting
for a wrong path should not contain one.
Raised by a reviewer on the PR.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test_internal_error_handler_is_registered read app.error_handler_spec, a Flask
internal whose shape has moved between releases, and proved only that something
was registered. A handler can be registered and still return HTML or log
nothing, so the test could not fail for either way the guarantee actually breaks.
Now driven through app.handle_exception(), Flask's own entry point for an
unhandled exception, asserting the response is JSON and that the request was
logged. Not done by registering a route that raises: the `app` fixture is
function-scoped but wraps a session-scoped Flask object, so the route would leak
into every later test and collide on re-registration. PROPAGATE_EXCEPTIONS is
forced off for the call because TESTING=True turns it on, which re-raises before
any handler runs.
The log assertion matches this handler's own wording rather than just the path.
Flask's log_exception() already emits "Exception on /path [GET]" before any
handler runs, so a path-only assertion passes even with the handler's logging
deleted — confirmed by mutation, which is how the first version of this test was
caught being vacuous. Both mutations (return HTML, drop the log) now fail, on
different assertions.
Raised by a reviewer on the PR.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Picks up serverkit-agent 219e75e, which points the usage and --help examples at
/api/v1/servers/install.{sh,ps1} instead of the domain root. Produced by
scripts/sync-agent-installers.sh, not hand-edited — scripts/test/check-installer-drift.sh
fails on any divergence and confirms these match the canonical copies again.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
User-facing half of the registry-mirror work: an offline panel (or one with the
registry disabled) was missing LocalKit Bridge from the Marketplace because the
bundled fallback had fallen behind the published index.
The sync script and the test guard are tooling, so they stay in git log --
except for the one line about the new check, which is the part a reader of this
file would care about.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A `bundled: true` entry promises an extension that ships inside the panel, so it
carries no source/sha256 -- which means every other check in this file skips it.
test_matches_published_index only proves the catalog agrees with the registry,
not that either agrees with the code, so a bundled entry could name a version
the panel does not ship, or an extension it does not ship at all, and nothing
would notice.
Checks each bundled entry against builtin-extensions/<slug>/plugin.json. Offline
and deterministic, so unlike the published-index comparison it fails on the PR
that introduces the mismatch rather than needing the network.
Only asserted in this direction: a builtin deliberately absent from the catalog
is a product decision, not rot.
Verified non-vacuous by mutation -- claiming a wrong version and naming a
non-existent bundled slug each fail, on different assertions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
backend/app/data/registry_index.json is a mirror of serverkit-extensions'
index.json, and nothing automated the mirror -- so every registry publish left
the panel's offline fallback stale until CI complained. That is how
`serverkit-localkit` sat missing from it: an offline panel offered a different
extension set than an online one.
Same shape as scripts/sync-agent-installers.sh: prefer a sibling checkout so it
works offline and before the registry PR is merged, else the published branch,
resolving the default branch from the API rather than hardcoding a name.
Two things it is deliberately careful about:
- It vendors the RAW index, never https://serverkit.ai/ext/index.json. The
panel fetches through that proxy at runtime and the proxy rewrites every
relative `logo` path to an absolute serverkit.ai URL, while the bundled
loader resolves relative paths itself. Vendoring the proxied copy would bake
in the wrong paths, and the tuple the tests compare (version/source/sha256)
is identical either way -- nothing would have caught it.
- It keeps the local `note` (which does not exist upstream) and writes with
indent=2/ensure_ascii=True/trailing newline, which round-trips the existing
file byte-for-byte. An in-sync run is a true no-op, not a reformat.
It refuses to write an index with no entries, so a broken or truncated canonical
source cannot blank the Marketplace fallback.
Verified: no-op when in sync; reproducing the real bug (dropping the localkit
entry) and re-running restores the file byte-identical to HEAD; repins are
reported; the empty-canonical guard exits non-zero leaving the file untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`app/data/registry_index.json` is the Marketplace's offline fallback and a
mirror of serverkit-extensions/index.json on main. The registry gained
`serverkit-localkit` (v0.2.0, bundled) and the mirror was never updated, so
an offline panel would have offered a different extension set than an online
one -- and test_registry_bundled_index::test_matches_published_index failed
on every run.
Copied the entry verbatim into its published position. The rest of the file
already matched published field-for-field; this is the whole delta.
`builtin-extensions/serverkit-localkit/` ships in the panel at the same
version, so `bundled: true` is correct and the entry carries no source/sha256.
Nothing automates this mirror, so it will drift again on the next registry
publish.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The vendoring and drift-check scripts hardcoded a branch name for the fallback
fetch. That was wrong twice in one day: first `main` (the agent repo used
`master`, so the fetch 404'd), then `master` (which is being renamed to `main`).
A hardcoded branch turns any rename into a 404 that surfaces as "cannot reach
the canonical copy" rather than "wrong branch name" — a confusing way to fail
for a check whose whole job is to be believed.
They now ask the GitHub API for the repo's declared default_branch, falling back
to probing main then master if the API is unreachable or rate-limited. AGENT_REF
still pins an explicit ref (a tag, say) when you want one. Verified: the API
currently reports `master`, and it will report `main` on its own once the rename
lands — no follow-up edit here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The vendoring and drift-check scripts defaulted AGENT_REF to `main`, which this
repo uses but the agent repo does not -- it has no `main` branch at all.
Verified: raw.githubusercontent.com/jhd3197/serverkit-agent/main/install.sh is
404, /master/install.sh is 200.
The bug was invisible locally because both scripts prefer a sibling
../serverkit-agent checkout and only fall back to fetching. It would have shown
up first in nightly CI, as "could not fetch the canonical copy" -- which reads
as an outage rather than a wrong branch name, so it is worth the comment.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
scripts/install.sh and scripts/install.ps1 are now VENDORED copies. The agent
repo owns them; this repo carries a copy only because the panel serves them off
its own disk at GET /api/v1/servers/install.sh|.ps1 and cannot link to another
repository.
Two copies of one file is what caused the second half of #101. When the agent
moved out of this monorepo it took its installer with it, and from then on there
were two -- actually four: the agent repo also carried a third, older pair under
its own scripts/ that nothing referenced. Each looked fine in isolation, nothing
ever compared them, and the panel went on serving one that downloaded from
`agent-v*` tags which exist in no repository. Unit tests are structurally blind
to this: they exercise whichever copy lives in their own repo and have no idea
another exists.
So:
scripts/sync-agent-installers.sh re-vendors from the agent repo (a sibling
checkout if present, else the published
branch), normalising to LF so this repo's
.gitattributes can apply its own EOL rule
scripts/test/check-installer-drift.sh fails, with the diff and the fix, if a
vendored copy differs from canonical
(EOL-insensitive: *.ps1 is checked out
CRLF here and LF there, which is not drift)
The drift check runs nightly alongside the release-chain verification, since
both need to reach GitHub. Both scripts run locally against a sibling checkout,
so this works offline and before an agent change is published.
Both installers carry a provenance header naming the canonical location, so
anyone who opens the vendored copy to patch it is told where to patch instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Also corrects the scope note, which said the agent is tagged agent-vX.Y.Z and
linked to agent/README.md -- a path that no longer exists. The agent ships from
jhd3197/serverkit-agent and is tagged vX.Y.Z there.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Issue #101 shipped because release.yml went straight from `build` to `push`
with nothing in between. The image was missing scripts/, so every containerised
panel 404'd its own enrollment URL, while the entire test suite stayed green --
pytest runs against the source tree, where the file always exists. Only booting
the artifact catches that class of bug.
scripts/test/smoke-docker-image.sh builds (or takes) an image, asserts the files
the panel reads at request time are present, boots it, and requests every route
that serves a file off disk. release.yml now builds amd64, runs it, and only
then pushes multi-arch (layers are cached, so the push reuses the build).
release-smoke.yml gets the same job on dev and PRs, and its path filters now
include Dockerfile, .dockerignore and scripts/ -- none of which used to trigger
it.
scripts/test/verify-agent-release-chain.sh covers the other half, which unit
tests structurally cannot: they stub curl, so they only ever prove the installer
is self-consistent, never that the URL it builds resolves to a real file. This
one reads the coordinates back out of the script, asks GitHub, downloads the
archive, verifies the checksum and confirms the tarball entry name the installer
moves. It runs nightly, and is also the tripwire for the reverse case -- the
agent repo changing its release naming and breaking the panel from the other
side of the repo split.
Both are plain scripts so the same check runs locally:
bash scripts/test/smoke-docker-image.sh
bash scripts/test/verify-agent-release-chain.sh
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Tracing issue #101 turned up that a panel failing to serve its own agent
installer left no diagnostic trail at all. The backend had exactly one error
handler -- the 404/SPA one -- no 500 or HTTPException handler, and nothing
anywhere configured logging, while only 2 of 100 app/api modules ever call
logger.error.
Adds:
- an errorhandler(500) that logs the unhandled exception with its method and
path and returns JSON. Flask routes here only when it is not propagating, so
pytest and the dev server still re-raise with a full traceback.
- an errorhandler(HTTPException) so framework-generated errors under /api/
(405 from the router, 413, abort()) return JSON instead of Werkzeug's HTML
page to clients that are parsing JSON. Non-API paths keep the HTML page, and
the more specific 404/500 handlers still win for those codes.
- _configure_logging(): a stderr handler with a timestamp/level/logger-name
formatter, level from LOG_LEVEL. Services across the codebase log through
logging.getLogger(__name__); with no root handler those records fell through
to logging's last-resort handler -- WARNING and above only, unformatted, INFO
dropped on the floor.
Two things the container run caught while writing this: Flask's default handler
has to come off app.logger or every record prints twice (once as %(module)s via
Flask's handler, once as %(name)s via ours, both reaching stderr through
propagation), and the level must not key off DEBUG alone -- TestingConfig sets
DEBUG=True, which would have moved the whole test suite to DEBUG logging as a
side effect. Both are pinned by tests.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The command the panel prints for enrolling a server was
`curl -fsSL <url> | sudo bash -s -- …`. In a pipeline the shell reports bash's
exit status, not curl's, so a failed download exits 0: the operator sees one
line of curl noise, the shell reports success, and nothing was installed.
Measured against a 404 -- `curl -fsSL <404> | bash -s -- --token TOK; echo $?`
prints 0; with `set -o pipefail` it prints 22.
Download to a file first and run it on success, so a failed fetch stops the
command and surfaces curl's non-zero exit. Applied to the Add Server drawer, the
server-detail connection modal and the Downloads page.
The Windows one-liner is left alone: `irm` writes an error and the pipeline
stops, so its failure is already visible.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three changes to the endpoints that serve the fleet-agent installers.
A missing installer answered 404. That is what sent issue #101's reporter
looking for a wrong URL -- they filed it as "incorrect paths in setup" -- when
the file had simply never been shipped into the image. The status code produced
the misdiagnosis. It is now 503, the response names the path that was searched,
and the same path is written to the panel log, because the response body does
not survive the `curl -f` the enrollment one-liner uses: curl discards it and
prints only "The requested URL returned error: 404". For an endpoint consumed by
curl, the status code and the server log are the only diagnostics that reach
anyone.
The panel's release lookup pointed at jhd3197/ServerKit and filtered for
`agent-v*` tags, which exist nowhere since the agent moved to its own repo, so
/api/v1/servers/agent/version returned 503 to every agent polling for an update.
It now uses AGENT_GITHUB_REPO (env SERVERKIT_AGENT_GITHUB_REPO) and a ^v(\d…)
tag filter that skips the agent repo's malformed "v" release.
_load_installer() is shared by the .sh and .ps1 routes so they cannot drift, and
SERVERKIT_SCRIPTS_DIR overrides the resolved directory for unusual layouts.
New tests cover the endpoints, the release lookup, and -- the one that would
have caught the original bug -- a packaging invariant that parses the
Dockerfile's COPY sources and asserts every installer the panel serves is
actually copied into the image. No endpoint test could have caught it: pytest
runs against the source tree, where the file always exists.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
When the Go agent moved out of this monorepo into jhd3197/serverkit-agent, the
installers did not follow it. They still asked jhd3197/ServerKit for `agent-v*`
tags -- coordinates that exist in no repository: jhd3197/ServerKit has zero
agent-v* releases, and the binaries are published as vX.Y.Z in the agent repo.
Version discovery therefore failed outright, and the download URL was a 404.
Everything downstream was already correct and is unchanged: the asset name
serverkit-agent-<ver>-linux-<arch>.tar.gz, the entry inside the tarball
(serverkit-agent-linux-<arch>) and the checksums.txt entry names all match the
real v1.2.0 release. Only the repo and the tag prefix had rotted.
Version discovery now requires a digit after the "v". The agent repo carries a
malformed release tagged literally "v", which would otherwise parse to an empty
version and build .../releases/download/v/serverkit-agent--linux-amd64.tar.gz.
test_agent_install.sh encoded the dead scheme in its fixtures (agent-v0.3.2), so
it stayed green throughout. It now pins the release coordinates as a contract:
the repo, the /v${VERSION}/ URL scheme, the absence of any live agent-v
reference, and the checksums URL exercised through the curl stub. All four fail
against the pre-fix script.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The panel serves the fleet-agent installers at /api/v1/servers/install.sh and
/install.ps1 by reading them off its own filesystem, resolving <tree root>/scripts
four dirnames up from app/api/servers.py. In the image that is /app/scripts --
and the Dockerfile only ever copied backend/, VERSION and frontend/dist, so the
directory did not exist. Every containerised panel answered the enrollment
one-liner printed in its own Add Server dialog with a 404, and no server could
join the fleet. Verified against the published image: `ls /app` has no scripts,
and a booted container returns 404 while /api/v1/system/health returns 200.
Bare metal was unaffected -- install.sh git-clones the whole repo, and the
release tarball excludes only scripts/test/output.
Ship just the two installers: both are standalone (they source nothing from
scripts/lib), so scripts/lib, keys, the test harnesses and stage-remote.sh stay
out of the image. The .dockerignore comment claiming the build "only needs
frontend/, backend/, VERSION" was wrong and authoritative, which is a large part
of why this went unnoticed; it now states the real list.
Closes#101 (packaging half).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
app/data/registry_index.json is what the Marketplace falls back to when the
live registry is unreachable, and what the test suite reads. Seven of its ten
entries carried a sha256 that does not match the artifact their own source URL
resolves to, at the same version: serverkit-gpu's zip hashes 3872d71f..., the
bundled copy claimed c861dafa.... A panel that fell back to this file would
download the correct, untampered release and reject it as tampered, on exactly
the panels already cut off from the network. It also sat nine entries and one
version behind (WordPress 1.0.0 against a published 1.0.1).
Nothing caught it because nothing checked it: the file was hand-maintained,
had no generator, and no test referenced it. A wrong-but-well-formed digest is
indistinguishable from a right one without fetching the artifact or the
published index.
Regenerated as a straight mirror of serverkit-extensions/index.json, and the
note now says it is a mirror and must not be hand-edited. The new test compares
the pinned (version, source, sha256) tuples against the published index and
skips only when the index is unreachable, so offline is not a failure but a
reachable mismatch is. Verified it fails against the old file, naming all seven
digests and the WordPress version, and passes against the regenerated one.
Also corrects the signed-releases roadmap line I marked done earlier today: the
verification path and the consent UX shipped, but no published release actually
carries a signature and the registry schema did not even document the field, so
every first-party install still takes the unsigned-consent path.
Four boxes were unchecked for things that have shipped. Signed extension
releases with install consent landed with plan 55's ed25519 signing service
and the marketplace consent modal. WordPress is out of the tree and installs
from the Marketplace like any other extension. The Minecraft extension, the
reusable gamekit framework and the connect card are all published at v1.0.0
and carried in the registry index.
SRV-record domain support is split out as the one game-server item that is
genuinely still open — grep finds no SRV handling in the extension — rather
than leaving it hidden inside a bullet whose other half was done. The
built-ins bullet now names only what is left, since the WordPress flagship it
called out has been extracted.
The python-provisioning job failed on ubuntu:22.04 with every apt lookup coming
back empty — software-properties-common not found, so the deadsnakes fallback
could not even be attempted and provision_python fell through to the source
build.
The installer was not broken. main()'s order is
identify_system -> choose_pkg_manager -> ensure_bootstrap_tools -> ... -> provision_python
and ensure_bootstrap_tools is what refreshes the package index — its own comment
says minimal images "ship with empty package indexes". This test called
provision_python without it, so on a fresh container the apt lists were empty
and the FIRST `apt-get install` had no candidate for anything. A test that calls
production functions out of order invents failures; "fixing" install.sh to
satisfy it would have made the real installer worse.
Why it was not caught locally: my verification ran `apt-get update` before the
test, while the CI step is `command -v bash || { apt-get update && install bash; }`
— Ubuntu already ships bash, so that update never runs. I had verified a machine
state CI does not have. Re-verified by running the CI command VERBATIM.
All eight matrix images, exact CI invocation, 7 passed / 0 failed each:
ubuntu:22.04 -> 3.12 (deadsnakes) debian:12 -> 3.11
ubuntu:24.04 -> 3.12 debian:13 -> 3.13
rockylinux:9 -> 3.12 almalinux:9 -> 3.12
fedora:40 -> 3.12 opensuse/leap:15.5 -> 3.11
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
write_config mints its secrets with `openssl rand -hex 32` and a python3
one-liner. The cross-distro containers install ONLY bash — ubuntu:24.04 and
debian:12 ship neither binary — so under `set -e` the failed command
substitution aborted write_config before it wrote a line, and three assertions
read an empty .env:
✘ stock install produced: []
✘ an exposed backend port still trusted X-Forwarded-For: []
✘ external proxy did not set 2 hops: []
The three T44 cases that passed are exactly the ones taking write_config's
existing-.env early return, which never reaches key generation — that split is
what identified the cause.
My bug, and the avoidable kind: T44 was the one part of this suite that leaned
on real binaries, in a file whose whole premise is that it stubs every external
tool. It passed locally only because a dev box happens to carry openssl and
python on PATH.
Stub both inside the three affected subshells. Shell functions take precedence
over PATH lookup, including through `"${PYTHON_BIN:-python3}"`, so no PATH
juggling is needed. The stub values are literals rather than built with
seq/tr/head — those are precisely the tools a stripped base image turns out not
to have, which is the bug being fixed.
Verified in the exact failing image: ubuntu:24.04 goes from 121 passed / 3
failed to 124 passed / 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`apt-get install -y` answers apt's own prompts but NOT debconf's. tzdata,
keyboard-configuration and friends still open an interactive dialog and block
forever. pkg_add captures apt's output, so the installer hangs in complete
silence — no prompt, no progress, nothing to diagnose.
Observed, not theorised: ubuntu:22.04 sat for 60 minutes on 39 bytes of output,
stuck in tzdata.postinst -> debconf frontend. Real Debian/Ubuntu servers are
exposed to the same stall; a container is just where it showed up. Under
`curl | bash` it is worse still, because stdin is the installer script itself,
so debconf can consume installer source as its answers.
DEBIAN_FRONTEND was set NOWHERE in install.sh, scripts/lib/pkg.sh or update.sh.
Every other manager was already guarded (dnf/yum -y, zypper --non-interactive,
pacman --noconfirm) — apt was the only unguarded one, and pkg.sh's own
docstring already claimed the whole function was non-interactive.
In pkg.sh it goes in ARGV as `env DEBIAN_FRONTEND=noninteractive …`, not an
export: _pkg_sudo runs plain `sudo "$@"` and sudo's env_reset would drop an
exported variable before apt saw it. --force-confold/--force-confdef cover the
other prompt class (modified conffiles), which DEBIAN_FRONTEND does not
suppress.
Tests: pkg_install must be non-interactive for every manager, and pkg_add's apt
branch is guarded statically. Three existing assertions pinned the exact apt
argv (`install -y <pkg>`) and now match the package instead — a flag added to
pkg_add should not fail tests whose subject is which package got installed.
Verified across the whole distro matrix: ubuntu:22.04 went from a 60-minute
hang to 7 passed in minutes; debian:13/12, ubuntu:24.04, fedora:40,
rockylinux:9 and opensuse/leap:15.5 all 7 passed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seven distro images were green throughout issue #99 because every one of them
runs the unit suites against STUBS. That is the right trade for logic, but it
left the one thing that talks to real package repos completely untested — so a
new Debian release dropping the Python we ask for could only ever be found by a
user.
scripts/test/test_provision_python.sh runs identify_system, choose_pkg_manager
and provision_python for real, then proves the result is usable: in range, can
create a virtualenv, has working pip, and imports ssl/sqlite3/ctypes. It needs
no systemd, nginx or ports, so it fits the plain containers the matrix already
pulls. Reaching the source build counts as a FAILURE, not a pass: a distro that
gets there is one we have not really added support for.
Wired up as a new `python-provisioning` job over the same images, and debian:13
added to both matrices. Adding trixie to the existing matrix alone would have
been decorative — those legs would have gone green on a distro that cannot
install.
Proven in both directions before being relied on: against the pre-fix tree it
reproduced #99 exactly on debian:13 (the reporter's apt errors, the fall
through to a source build, and the abort message), while debian:12 passed 7/7.
Also fixed the suite pinning the bug. T2 asserted `ver_in_range 3.13` must be
FALSE and T1's stub used 3.13 as its "too new" case, so the gate could not be
widened without the tests failing. Both moved to 3.14, plus a direct guard on
PYTHON_MAX — otherwise a regression to 3.12 could be "fixed" by editing the
assertion to match. 122 passed, 0 failed.
full-stack-test.ps1 documents why debian13 is NOT in the VM harness: no trixie
box publishes a Hyper-V provider (generic/debian13 does not exist,
debian/trixie64 is libvirt-only, bento/debian-13 has no hyperv) and Multipass
is Ubuntu-only. Named there with what remains uncovered on trixie — the full
systemd + nginx + API end-to-end — rather than left as a silent gap.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Issue #99. Three separate bugs, and the apt errors the reporter pasted were the
least of them.
1. The gate. Debian 13 (trixie) ships ONLY python3.13 — no 3.11, no 3.12 — so
PYTHON_MAX=3.12 could never be satisfied from its repos. Ceiling raised to
3.13; locate_python now probes python3.13 first. Ubuntu 26.04 lands in the
same place.
2. The Debian branch asked for versions by name (python3.11, else python3.12),
which is exactly why trixie broke it. It now installs the distro's DEFAULT
python3 first — that tracks the release, bookworm 3.11, trixie 3.13 — and
falls back to explicit versions only for releases whose default is out of
range. Hardcoding 3.13 instead would have relocated the same trap to Debian
14. The probe is locate_python, not `command -v`: a binary existing says
nothing about it being in range or venv-capable.
3. The source-build fallback could never have worked. It ran ./configure and
make while NOTHING in install.sh ever installed a compiler — only the -dev
headers — so on any clean VPS it failed immediately, and every step was
piped to `tail -1` so the real error was discarded. That, not the apt
errors, is what actually ended the reporter's install.
Now: build-essential / gcc+make installed for the right family, an explicit
"no compiler" halt with the command to fix it, each step status-checked with
the log tail printed on failure, and the built interpreter verified to have
ssl, sqlite3 and ctypes (a source build silently omits modules whose headers
were missing, and both failures surface far from here). Dropped
--enable-optimizations: PGO triples a build that is already the last resort,
for runtime speed a control panel never notices.
Verified against real distro repos, not stubs — provision_python run for real
in each container: debian:13 now picks python3.13 (7 passed, was 2 failed and
falling through to the source build), debian:12 python3.11, ubuntu:24.04 and
fedora:40 python3.12, all 7 passed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Required for Debian 13 (issue #99), which ships only python3.13. SQLAlchemy
2.0.23 installs there and then dies at import:
AssertionError: Class <class 'sqlalchemy.sql.elements.SQLCoreOperations'>
directly inherits TypingOnly but has additional attributes
{'__static_attributes__', '__firstlineno__'}
3.13 adds those two class attributes and SQLAlchemy's TypingOnly guard rejected
anything it did not expect. Fixed upstream in 2.0.31.
Measured rather than assumed, because this pin sits under every query in the
app. Full backend suite on Python 3.11 with 2.0.52: 3542 passed, 48 skipped —
byte-identical to the same suite on 2.0.23. Run against a throwaway overlay so
the comparison was pins-only, nothing else moved.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SystemService._is_iana_timezone and CronService._same_application were both
declared as
@classmethod
@staticmethod
def _method(...)
which is meaningless and only ever worked because Python 3.9 let classmethod
chain to another descriptor. That chaining was deprecated in 3.11 and REMOVED
in 3.13, so cls stops being absorbed and leaks in as the first argument:
TypeError: SystemService._is_iana_timezone() takes 1 positional argument
but 2 were given
Latent on the 3.11 we ship today, fatal on 3.13. It takes out server-time
display, every cron-to-application operation (suspend/resume/clear/jobs_for)
and, through those, application soft delete and restore.
Plain @staticmethod is correct on every version — calling a staticmethod
through `cls.` has always been fine — so no call site changes.
These were the ONLY two occurrences in the tree. Found while establishing
whether the stack runs on Python 3.13 for Debian 13 (issue #99): they were the
entire delta between 3.13 and 3.11, 8 failures across 4 files. With them fixed
those files are 45 passed on both interpreters.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
install.sh wrote no TRUST_PROXY_HEADERS line at all, so the code default (False)
applied while every host install serves the panel through its own nginx. Per-IP
rate limits, the login brute-force throttle and every audit-log entry therefore
keyed on 127.0.0.1, which quietly weakened all three -- one abusive client shared
a bucket with everyone else. .env.example claimed true, so the two disagreed and
it looked configured.
Trusting X-Forwarded-For is only safe when nothing can reach gunicorn without
traversing a proxy we control, so the switch follows that condition rather than
assuming it. On a host install it holds: render_service_unit binds gunicorn to
127.0.0.1 and bootstrap_firewall opens SSH + 80/443 + PANEL_PORT and never the
backend port. backend_bind_host() is now the single source of truth for both the
systemd unit and the trust decision so the two cannot drift, and
backend_is_loopback_only() gates the setting.
- stock install: TRUST_PROXY_HEADERS=true, TRUSTED_PROXY_HOPS=1
- SERVERKIT_EXTERNAL_PROXY=1: true with 2 hops, unchanged
- SERVERKIT_BIND_HOST not loopback: false plus a warning. The port is directly
reachable there, so the header is attacker-chosen, and a forged IP is worse
than a wrong one because it defeats the throttles that consume it.
Re-runs backfill it into installs that predate this, via _env_default_key, which
fills a gap but never overrides an uncommented value -- flipping someone's
deliberate false back on would be the worst kind of surprise.
.env.example moves to false on purpose: that file is the Docker/manual path and
docker-compose.yml publishes 5000 on all interfaces, so true was the unsafe
default there. Host installs never read it. SECURITY.md claimed "the shipped
nginx deploy sets it", which was untrue until now.
T44 covers all six behaviours, each mutation-checked by line number: disabling
the branch, inverting it to trust an exposed port, and disabling the backfill
each fail exactly one assertion and no others.
Shell suite 121 passed. Backend suite 3542 passed, 48 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Issue #96: an operator fronting the panel with Caddy and a smallstep private CA
hit a redirect loop, and there was no documented path to follow. The
architecture already tolerated this deployment -- the fix was a config file we
already ship -- but nothing said so anywhere.
Adds an INSTALLATION.md section covering both topologies (external proxy ->
ServerKit nginx, full features, HOPS=2; external proxy -> the app directly,
panel-only, HOPS=1), why the loop happens, how to count TRUSTED_PROXY_HOPS,
Caddy (with acme_ca for a private CA), Traefik, Nginx Proxy Manager and plain
nginx snippets, what a private CA does and does not require (the panel never
sees it; a remote agent's host must trust the root), and a symptom-to-cause
table for the loop, stale HSTS, the rejected websocket and wrong client IPs.
Also corrects the Docker documentation, which had drifted badly:
- README (en/es/pt/zh-CN), INSTALLATION and DEPLOYMENT all told people to run
`docker compose up -d` and reach the panel at http://localhost. It starts the
all-in-one container on :5000, and a containerised panel cannot manage its own
host -- now stated up front rather than discovered.
- DEPLOYMENT is a host-install guide that told operators to run
`docker compose build && up` to fix a panel, which would build a second panel
beside theirs, and carried a certbot cron copying certs and restarting a
container that does not exist on a host install.
- CLAUDE.md, AGENTS.md, CONTRIBUTING.md and LOCAL_DEVELOPMENT.md (x3) pointed at
a docker-compose.dev.yml that has never existed; the snippet offering to
create it referenced a frontend/Dockerfile.dev that does not exist either.
Every anchor introduced here was checked to resolve.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Issue #96's redirect loop came from the nginx site we install, not the app --
but nothing pinned "the app never redirects to HTTPS", so the docs relied on a
property no test kept true.
Covers: no redirect to https from the curated entry routes, the same under
X-Forwarded-Proto: https (the header any force-HTTPS middleware would key on),
and a sweep of every no-argument GET rule in the URL map so new blueprints are
covered without anyone remembering to add them; SERVERKIT_PUBLIC_URL auto-allowed
as an origin, trailing slash normalised, additive to CORS_ORIGINS and never '*';
that value reaching Socket.IO with async_mode='threading'; and the handshake
accepting the public origin while rejecting a stranger.
Mutation-checked rather than merely green: injecting a before_request HTTPS
redirect fails the sweep on all 352 routes it walks, and the handshake's 400
flips to 200 when the same origin is allowlisted.
ProxyFix on/off and HSTS gating are deliberately not duplicated here -- they are
already covered by test_trusted_client_ip.py and test_security_headers.py.
The config fixture loads config.py as a throwaway module rather than calling
importlib.reload. reload looks equivalent and is a trap: it rebinds
config.TestingConfig to a new class object while app/__init__.py still holds the
original config dict from its `from config import config`, so tests that
monkeypatch TestingConfig patch the new class while create_app reads the old one
and ProxyFix silently never turns on. That broke 9 tests in
test_trusted_client_ip.py and test_login_bruteforce.py, in a full run only.
Full suite: 3542 passed, 48 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Make "someone else terminates TLS in front of the panel" a supported install
shape instead of something you reverse-engineer from this script (issue #96).
SERVERKIT_EXTERNAL_PROXY=1 implies SERVERKIT_SKIP_SSL=1, so the plain-HTTP nginx
site is selected and there is no 301 for the external proxy to follow back into
itself; it also writes TRUST_PROXY_HEADERS=true and TRUSTED_PROXY_HOPS=2 (their
proxy appends the real client, our nginx appends their proxy) and asks for
SERVERKIT_PUBLIC_URL, deriving it from PANEL_DOMAIN when it can and warning
rather than blocking when there is no tty. Without that URL the Socket.IO
handshake is rejected on origin and the UI loads but never live-updates.
It applies on re-runs too, via a new _env_set_key helper. "Convert my install to
sit behind Caddy" is a re-run and lands in the existing-.env branch, which
previously only refreshed the ssl mode and profile -- so the documented
conversion path would have silently done nothing. A plain re-run never removes
proxy settings someone configured by hand.
SERVERKIT_CONFIG names a KEY=VALUE file of defaults for the same variables. It
is PARSED, not sourced: sourcing would let file assignments overwrite the
environment, inverting the documented precedence, and would execute anything
else in the file as root. Applying keys one at a time and skipping names already
set gives the stated semantics safely.
scripts/test/test_install.sh T43 covers all of it, including that a command
placed in the config file does not run, that the converted .env flips
SERVERKIT_SSL_MODE to insecure (config resolves the env var before the file, so
writing /etc/serverkit/ssl-mode alone would leave HSTS on), and that a default
install is untouched. T12 gained guards for the compose and Dockerfile
regressions.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
serverkit.conf and serverkit-insecure.conf both declare
`listen 80 default_server`, and both end by including
/etc/nginx/serverkit-conf.d/*.conf. write_config dropped canonical-domain.conf
into that directory with its own `listen 80 default_server` whenever
PANEL_DOMAIN was set, and nginx rejects that outright:
[emerg] a duplicate default server for 0.0.0.0:80 in
/etc/nginx/serverkit-conf.d/canonical-domain.conf:2
Reproduced against real nginx in BOTH ssl modes. main() runs configure_nginx ->
write_config -> launch_services, and launch_services starts nginx after the bad
file exists, so the failure lands during the install -- and install.sh runs
`nginx -t` nowhere, so nothing said why. Nothing ever removed the file either.
Use a regex server_name instead. nginx resolves exact names, then wildcards,
then regexes, and only then the default server, so this block still wins for a
bare-IP Host while every other host falls through to the panel. That also drops
the `if`, which nginx documents as best avoided.
Verified end to end against real nginx: `nginx -t` passes; Host: 203.0.113.9 ->
301 to the panel domain; Host: panel.example.com -> 200 panel; any other host ->
200 panel.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two independent bugs, both surfaced by issue #96.
The image died on every run with "sqlite3.OperationalError: unable to open
database file": config.py defaults DATABASE_URL to
sqlite:////app/instance/serverkit.db, the Dockerfile never created
/app/instance, and SQLAlchemy will not create a parent directory. Creating it
before the USER switch also fixes named-volume ownership -- Docker seeds an
empty volume from the image directory, permissions included, but creates it
root-owned when the path is absent from the image.
Separately, every docker-compose.yml service sat behind the legacy-frontend
profile, so `docker compose up -d` started nothing at all while the README and
install guide both gave that command as the Docker install. Following the
profile hint yielded the legacy nginx container alone -- a static file server
plus a proxy to a backend on the host -- serving an empty bind mount, which is
the 403 the reporter saw. Default `up -d` now runs the all-in-one image on
:5000 with a named volume; the legacy service stays profiled and says in a
comment that it needs a host install.
DATABASE_URL is pinned rather than interpolated. .env.example shipped a
*relative* sqlite path, which resolves against the container WORKDIR instead of
the volume and is destroyed by the next `compose down`; anyone who copied that
.env would inherit the trap. SERVERKIT_DATABASE_URL is the documented override.
Verified against a real build: boots healthy, serves the SPA, survives a
container recreate on the volume with 143 tables intact, and comes up with no
.env present at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Found by the full-stack VM harness, which 3524 unit tests and a 7-image
container matrix had all missed. On a real Ubuntu 24.04 install the log read
"Opening HTTP/HTTPS (80, 443) via nftables" and not one line of
bootstrap_firewall appeared: it returned at its "a firewall is already active"
guard, so the box got its two ports opened and no default-deny policy at all.
The headline feature of that work was a no-op on exactly the population it was
written for, and it looked green the whole way.
The guard asked `[ "$(firewall_detect)" != "none" ]`. firewall_detect answers a
different question — "which backend do I open a port THROUGH" — and returns
`nftables` whenever the nft binary exists and can list a ruleset, including an
empty one. That is true of every modern Linux, so the guard reported every box
as already protected. Verified directly against a stock-image fixture: same
inputs, `firewall_detect` -> nftables, inbound-default-deny -> unprotected.
firewall_inbound_default_deny() asks what the bootstrap actually cares about:
does this host already deny inbound by default? Presence of rules is
deliberately NOT the test -- Docker installs plenty of nft and iptables chains
and protects nothing. A default-deny policy on the input hook is: firewalld
running, ufw active *with* "deny (incoming)", an nft base chain hooked to input
with policy drop or reject, or an iptables INPUT policy that is not ACCEPT.
Unknown answers 1 (unprotected), because the caller's response is to offer
protection and every caller re-checks before acting.
firewall_detect is left alone -- configure_firewall legitimately wants a
backend name for a binary that exists -- and both functions now carry a comment
saying which question they answer, since that is what was conflated.
Six cases pin the behaviour, five of them shapes the old guard got wrong: an
empty nft ruleset must run the bootstrap, Docker's chains must not read as
protection, an active-but-allow-inbound ufw must be fixed rather than
respected, a real nft policy drop must be left alone, and the two predicates
must disagree on a stock image -- that last one asserted at the predicate level
so the distinction survives a refactor of either function. make_stub_stdout is
new because an exit-code stub cannot express "the tool ran fine and reported
nothing", which is the whole stock-image case.
Green on all seven CI images (265/265/265/275/275/275/263, 0 failed).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three entries for what shipped across this round, written from the operator's
point of view and without internal plan numbers per the file's convention: a
failed deploy no longer stopping the live site, the health doctor covering the
whole fleet with repair buttons that work, and the extension trust view that is
candid about which permissions the panel can and cannot verify.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Juan Denis