feat(images): mark platform-curated images and seed the lobby

The create-server form has no way to tell a user which of the whitelisted
images is a sensible starting point. Add 'recommended' as a third
image_whitelist.source alongside 'built' and 'external', and seed it with
the one image that has earned it.

The marker is presentation only. ImageAdmitted still turns solely on
enabled, so a recommended row is admitted by exactly the rule that governs
every other row and carries no extra privilege; a test pins both halves,
because the failure modes are silent and opposite — make admission
source-aware and the curated images vanish from the form, or let curation
bypass the disable switch and an admin who pulled an image finds it still
creatable.

Only one image is seeded, and the restraint is the point. Velocity runs
proxy-wide modern forwarding, so a backend that cannot verify the signed
handshake rejects every login the proxy sends it. The operator injects
FELIS_FORWARDING_SECRET into every backend but cannot make an image consume
it. An arbitrary public Minecraft image therefore passes admission, builds,
schedules, reports Ready — and then refuses every join, with nothing in the
server's status explaining why. Exactly two images read that variable,
deploy/limbo and deploy/lobby; limbo is the login gate and is nonsense as a
base for a user's server, which leaves lobby. The list grows when Felis
ships another forwarding-aware image, not before.

AdmitBuiltImage now preserves a 'recommended' source through its ON CONFLICT
path. Rebuilding a curated tag is the expected way to patch it, and that
rebuild arrives through this exact path, so a blind SET source = 'built'
would demote the curation on the first rebuild with nothing in the request
saying so. AddExternalImage deliberately does not preserve it: an admin
POSTing the ref is an explicit, named re-admission, and the 201 body reports
the Image it constructed without re-reading the row, so a sticky source
there would report a value the database does not hold.

The migration is idempotent via ON CONFLICT DO NOTHING, so an admin who
disabled or re-pointed the row does not have that decision undone on the
next apply.
This commit is contained in:
flyemoji committed 2026-07-20 14:31:14 +09:00
1 parent d26acc20ae
commit f36d5b87f6
4 files changed
+139 -2

No files matched your search

+19 -1
View File
@@ -78,10 +78,28 @@ const (
JobFailed
)
// image admission sources (spec §6 image_whitelist.source).
// image admission sources (spec §6 image_whitelist.source). The column is plain
// text, not an enum, so a curated value costs no schema change.
const (
SourceBuilt = "built"
SourceExternal = "external"
// SourceRecommended marks a platform-curated whitelist entry: an image Felis
// itself ships and vouches for, which the create-server form may surface ahead
// of the rest. It is a PRESENTATION marker only — admission still turns solely
// on enabled (see ImageAdmitted), so a recommended row is admitted by exactly
// the same rule as any other and carries no extra privilege.
//
// The bar for this marker is joinability, not popularity. Velocity runs
// proxy-wide modern forwarding, so a backend that cannot verify the signed
// handshake rejects every login the proxy sends it; only an image whose
// entrypoint consumes FELIS_FORWARDING_SECRET is actually reachable by a
// player (see operator.buildEnv, which injects it into every backend but
// cannot make an operator-typed Dockerfile read it). Recommending an arbitrary
// public Minecraft image would therefore ship a trap: it builds, schedules,
// and goes Ready, then refuses every join. Only the images Felis builds from
// deploy/ clear that bar — see 0018_recommended_images.sql for which, and why
// the honest set is one image rather than several.
SourceRecommended = "recommended"
)
// ErrNotFound is returned when a build id / image ref does not exist.
+32
View File
@@ -529,3 +529,35 @@ func TestImageAdmitted(t *testing.T) {
}
}
}
// A 'recommended' row (0018) is curation, not capability: it must be admitted by
// exactly the rule that governs every other source, and it must not become a way
// to bypass the disable switch. Both halves are pinned here because the failure
// modes are silent and opposite — make admission source-aware in one direction
// and the curated images quietly vanish from the create-server form; in the
// other, a disabled recommendation stays creatable after an admin pulled it.
func TestRecommendedImageAdmittedLikeAnyOtherSource(t *testing.T) {
b, st, _ := newBuilder()
st.images["felis-lobby:demo"] = Image{
ImageRef: "felis-lobby:demo", Source: SourceRecommended, Enabled: true,
}
st.images["felis-lobby:pulled"] = Image{
ImageRef: "felis-lobby:pulled", Source: SourceRecommended, Enabled: false,
}
admitted, err := b.ImageAdmitted(context.Background(), "felis-lobby:demo")
if err != nil {
t.Fatalf("ImageAdmitted: %v", err)
}
if !admitted {
t.Error("an enabled recommended image must be admitted; curation must not cost admission")
}
admitted, err = b.ImageAdmitted(context.Background(), "felis-lobby:pulled")
if err != nil {
t.Fatalf("ImageAdmitted: %v", err)
}
if admitted {
t.Error("a disabled recommended image must not be admitted; curation is not a disable bypass")
}
}
+26 -1
View File
@@ -128,12 +128,24 @@ func (s *PGStore) ListUnfinishedBuilds(ctx context.Context) ([]Build, error) {
// AdmitBuiltImage upserts the whitelist row on scan-gate success. ON CONFLICT
// re-enables and re-stamps a previously-removed or superseded ref, so a rebuild
// of the same tag re-admits it (spec §16).
//
// source is the ONE column the conflict path does not overwrite unconditionally:
// a 'recommended' row is platform curation (0018), while every other field here
// describes the build that just succeeded and must win. Rebuilding a curated tag
// is the expected way to patch it, and that rebuild arrives through this exact
// path — so a blind `SET source = 'built'` would silently demote the curation on
// the first rebuild, with nothing in the request saying so. Preserving it keeps
// the marker a deliberate admin decision: DELETE /images is the way to clear it,
// not a build completing. Only 'recommended' is sticky; an 'external' row still
// becomes 'built', because a real build genuinely supersedes a hand-pushed ref.
func (s *PGStore) AdmitBuiltImage(ctx context.Context, img Image) error {
const q = `INSERT INTO image_whitelist
(image_ref, source, build_id, added_by, enabled, added_at)
VALUES ($1, 'built', NULLIF($2, ''), $3, true, $4)
ON CONFLICT (image_ref) DO UPDATE
SET source = 'built', build_id = EXCLUDED.build_id, added_by = EXCLUDED.added_by,
SET source = CASE WHEN image_whitelist.source = 'recommended'
THEN 'recommended' ELSE 'built' END,
build_id = EXCLUDED.build_id, added_by = EXCLUDED.added_by,
enabled = true, added_at = EXCLUDED.added_at`
_, err := s.db.ExecContext(ctx, q, img.ImageRef, img.BuildID, img.AddedBy, img.AddedAt)
return err
@@ -163,6 +175,19 @@ func (s *PGStore) ListImages(ctx context.Context) ([]Image, error) {
return out, rows.Err()
}
// AddExternalImage upserts a hand-pushed ref. Unlike AdmitBuiltImage this path
// does NOT preserve a 'recommended' source, and the asymmetry is deliberate: an
// admin POSTing this exact ref is an explicit, named re-admission, not a build
// completing behind their back, so demoting the curated row is the outcome they
// asked for. It also keeps the 201 body honest — AddExternalImage returns the
// Image it constructed (source=external) without re-reading the row, so a sticky
// source here would report a value the database does not hold.
//
// Note that the demote branch is unreachable for the ONLY recommended row Felis
// currently seeds: the caller validates with ValidateImageRef first, which refuses
// a bare local containerd tag, and 0018's felis-lobby:demo is exactly that. The
// branch is written for the host-qualified recommendations this list grows into,
// not for today's single seed.
func (s *PGStore) AddExternalImage(ctx context.Context, img Image) error {
const q = `INSERT INTO image_whitelist
(image_ref, source, added_by, enabled, added_at)