feat(files): add the server file editor
Give an owner a way to repair the one failure no other endpoint covers: a
server that will not boot because a single line of server.properties or a
plugin's YAML is wrong. Until now that needed a human with cluster access.
felis-api cannot touch a world in-process — the world PVC is ReadWriteOnce
and its lifecycle belongs to the operator's StatefulSet — so the work runs
as a one-shot Job, and the server must be stopped first because a running
one holds the volume. That is the same constraint that shapes restore and
backup, and the handlers enforce the stopped gate the same way.
What is different is that the caller wants the OUTPUT, not just the side
effect. The Job prints its result to stdout and felis-api reads it back
through the pods/log subresource, which needs no permission felis-api does
not already hold: jobs:create, pods:list, pods/log:get. No pods/exec, no
pods/portforward, not even pods:get. The price is latency — every operation
is a Pod schedule — which is why this is a repair tool and not a file
manager.
Containment is structural, not textual. Every filesystem access goes through
os.Root, the stdlib's escape-proof directory handle, which resolves each
component against the open root descriptor and refuses "..", absolute paths,
and symlinks leading outside. The string-prefix check used elsewhere is not
reused here: it validates a path as text and then opens it as a path, and a
world directory holds attacker-influenced content, so a symlink swapped in
between those two steps is a live threat rather than a theoretical one.
os.Root has no such window because the check and the open are one operation.
The Job's isolation is a strict subset of a restore Pod's: the weak
felis-restore SA with its token auto-mount disabled, exactly one volume (the
world PVC, mounted read-only for list and read so two of the three
operations cannot mutate anything), no Secret, no ConfigMap, no database
URL, non-root with an fsGroup matching the operator's so a written file is
readable by the server that later mounts it, and backoffLimit 0 so a failed
write is never silently retried as a second write.
Two limits on the surface are worth stating plainly, because the mount is
the server's whole working directory rather than a config subtree:
* A write accepts arbitrary bytes at any path, so an owner can place a
loadable plugin jar. This is deliberate — it is what a hosting panel's
file manager does, scoped to a server the caller already owns and
already drives through /command — but it is the one owner-tier route
that lands executable code in a backend pod, since images are
admin-only and modpack submissions need an admin verdict.
* config/paper-global.yml is refused on read. felis-lobby's entrypoint
writes FELIS_FORWARDING_SECRET into it on every boot, and that value is
identical on every backend, so reading it from a server you own would
hand you the handshake key for everyone else's. It is the only path in
the mount that is not the caller's own data, and therefore the only
denial. The comparison is on the cleaned path, or ./config/... would
walk straight through it.
Writing that file is still allowed: it leaks nothing, and the entrypoint
rewrites it whole on every boot regardless.
The write body's content field is a *[]byte rather than a []byte for the
reason permissionRequest.Value is a *bool — a plain slice makes absent,
null, and empty indistinguishable, so a body of {} would decode to nil and
truncate the target to zero bytes while answering 200, destroying the very
config the caller opened the editor to repair.
This commit is contained in:
14 files changed
+2994
No files matched your search
@@ -63,6 +63,14 @@ type API struct {
|
||||
// authorization boundary is exercised before the backup-Job executor is wired.
|
||||
Backuper Backuper
|
||||
|
||||
// Files is the server file editor (list / read / write a file in a stopped
|
||||
// server's world volume — the "one wrong line in server.properties" repair).
|
||||
// Like Restorer and Backuper it is optional: when nil the file routes report
|
||||
// 503, so the owner-or-admin and stopped gates are exercised before the
|
||||
// file-Job executor is wired. Unlike them its calls are synchronous, because
|
||||
// the caller wants the listing or the bytes back, not a 202.
|
||||
Files FileEditor
|
||||
|
||||
// Submissions is the user-modpack approval lane (a user-directed extension over
|
||||
// the §16 build subsystem; see internal/submit). It is optional: when
|
||||
// nil the /me/submissions and /submissions routes report 503 rather than 404, so
|
||||
@@ -401,6 +409,24 @@ func (a *API) externalAPIRoutes() []apiRoute {
|
||||
{Method: "GET", Pattern: "/api/v1/backups", h: a.handleListBackups},
|
||||
{Method: "POST", Pattern: "/api/v1/servers/{name}/restore-backup", h: a.handleRestoreBackup},
|
||||
{Method: "POST", Pattern: "/api/v1/servers/{name}/backup", h: a.handleBackupNow},
|
||||
// Server file editor: list / read / write a file in a STOPPED server's world
|
||||
// volume (handlers_files.go). App-tier, exactly like the backup pair above and
|
||||
// for the same reason — every route gates on owner-or-admin inside the handler,
|
||||
// so an owner repairs their own broken server without an admin's Zero-Trust
|
||||
// path. The path travels as ?path= rather than a segment because a file path
|
||||
// contains '/' (the same reason DELETE /images takes ?ref=). {name}/files is
|
||||
// the directory face; {name}/file is the single-file face.
|
||||
//
|
||||
// "Config editor" undersells the surface, so be precise about what app-tier
|
||||
// now reaches: the mount is the server's WHOLE working directory, not a
|
||||
// config subtree, so a write can place a loadable plugin jar (a deliberate
|
||||
// capability — see the op list in fileedit/exec.go) and a read can pull any
|
||||
// file in it. Exactly one path is denied, config/paper-global.yml, because it
|
||||
// holds the cluster-wide forwarding secret and is therefore the one thing in
|
||||
// the mount that is not the caller's own data (fileedit.secretConfigPath).
|
||||
{Method: "GET", Pattern: "/api/v1/servers/{name}/files", h: a.handleListFiles},
|
||||
{Method: "GET", Pattern: "/api/v1/servers/{name}/file", h: a.handleReadFile},
|
||||
{Method: "PUT", Pattern: "/api/v1/servers/{name}/file", h: a.handleWriteFile},
|
||||
// Account linking (spec §10), web side: /start reports link status (it is the
|
||||
// pointer handleClaim's 412 emits), /verify consumes the in-game code and binds
|
||||
// the account. App-tier, not admin — linking your own account is an ordinary
|
||||
|
||||
@@ -0,0 +1,247 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"net/http"
|
||||
|
||||
"felis.lolicon.best/internal/apis/felis/v1alpha1"
|
||||
"felis.lolicon.best/internal/fileedit"
|
||||
"felis.lolicon.best/internal/naming"
|
||||
)
|
||||
|
||||
// FileEditor is the server-file-editor surface the API depends on: list a
|
||||
// directory, read a file, write a file, all inside one server's world volume. It
|
||||
// is the repair lever for the case no other endpoint covers — a server that will
|
||||
// not boot because one line of a config file is wrong.
|
||||
//
|
||||
// Like Restorer and Backuper it only describes the operation, never the mechanism:
|
||||
// felis-api cannot touch a world in-process (the world PVC is ReadWriteOnce and
|
||||
// owned by the operator's StatefulSet), so the production implementation hands off
|
||||
// to a one-shot Job and reads the result back through pods/log — see
|
||||
// internal/fileedit, which explains why that transport needs no RBAC felis-api
|
||||
// does not already hold. Unlike Restorer and Backuper these calls are
|
||||
// SYNCHRONOUS: the caller wants the listing or the bytes, so the handler blocks on
|
||||
// the Job (seconds, dominated by Pod scheduling) rather than answering 202.
|
||||
//
|
||||
// It is an interface so the handlers are unit-tested against a fake; the
|
||||
// production implementation is *fileedit.Editor. Using fileedit.Entry directly
|
||||
// mirrors how ImageBuilder uses build.Request/build.Image rather than restating a
|
||||
// parallel type on this side of the seam.
|
||||
//
|
||||
// It returns fileedit.ErrNotFound / ErrBadPath / ErrTooLarge for caller-fault
|
||||
// failures, which writeFileEditError maps to 404 / 400 / 413.
|
||||
type FileEditor interface {
|
||||
List(ctx context.Context, server, path string) (entries []fileedit.Entry, truncated bool, err error)
|
||||
Read(ctx context.Context, server, path string) ([]byte, error)
|
||||
Write(ctx context.Context, server, path string, content []byte) error
|
||||
}
|
||||
|
||||
// writeFileRequest is the PUT /servers/{name}/file body. Content is []byte, so
|
||||
// encoding/json requires it to be base64 — which is what makes the write path
|
||||
// binary-safe: a config file with CRLF line endings, a UTF-8 BOM, or a stray
|
||||
// non-UTF-8 byte round-trips intact instead of being mangled by a string decode.
|
||||
//
|
||||
// It is a *[]byte, NOT a []byte, for the same reason permissionRequest.Value is a
|
||||
// *bool: a plain slice makes "absent", "null", and "" indistinguishable, so a body
|
||||
// of {} would decode to nil and TRUNCATE the target file to zero bytes while
|
||||
// answering 200 — a client serialisation bug silently destroying the very config
|
||||
// the caller opened this endpoint to repair. nil now means "the field was omitted"
|
||||
// and is refused; an explicit "" is still a legitimate deliberate truncate.
|
||||
type writeFileRequest struct {
|
||||
Content *[]byte `json:"content"`
|
||||
}
|
||||
|
||||
// handleListFiles serves GET /api/v1/servers/{name}/files?path=… — one directory's
|
||||
// entries inside the server's world volume. An absent or empty path lists the
|
||||
// world root.
|
||||
//
|
||||
// The path travels as a QUERY parameter, not a path segment, for the same reason
|
||||
// handleRemoveImage takes ?ref=: a file path contains '/' and does not round-trip
|
||||
// through a single {placeholder}. It is passed to the executor unmodified — this
|
||||
// handler deliberately performs NO path validation, because the only containment
|
||||
// that can be trusted is the one applied at the moment of opening the file, inside
|
||||
// the Job, by os.Root (see fileedit.Execute). A pre-validating handler would
|
||||
// invite exactly the false confidence that makes string-prefix containment fail.
|
||||
func (a *API) handleListFiles(w http.ResponseWriter, r *http.Request) {
|
||||
name, ok := a.authorizeFileOp(w, r)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
path := r.URL.Query().Get("path")
|
||||
|
||||
entries, truncated, err := a.Files.List(r.Context(), name, path)
|
||||
if err != nil {
|
||||
writeFileEditError(w, r, err)
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, map[string]any{
|
||||
"path": path, "entries": entries, "truncated": truncated,
|
||||
})
|
||||
}
|
||||
|
||||
// handleReadFile serves GET /api/v1/servers/{name}/file?path=… — one file's bytes,
|
||||
// base64-encoded by encoding/json's []byte handling. Reading is capped at
|
||||
// fileedit.MaxReadBytes inside the Job; an oversized file is 413, not a truncated
|
||||
// read, because a config editor that silently returned half a file would let a
|
||||
// subsequent save destroy the other half.
|
||||
func (a *API) handleReadFile(w http.ResponseWriter, r *http.Request) {
|
||||
name, ok := a.authorizeFileOp(w, r)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
path := r.URL.Query().Get("path")
|
||||
if path == "" {
|
||||
writeError(w, r, newError(http.StatusBadRequest, "bad_request",
|
||||
"the ?path= query parameter is required"))
|
||||
return
|
||||
}
|
||||
|
||||
content, err := a.Files.Read(r.Context(), name, path)
|
||||
if err != nil {
|
||||
writeFileEditError(w, r, err)
|
||||
return
|
||||
}
|
||||
writeJSON(w, http.StatusOK, map[string]any{"path": path, "content": content})
|
||||
}
|
||||
|
||||
// handleWriteFile serves PUT /api/v1/servers/{name}/file?path=… — replace a file's
|
||||
// contents, creating the file if absent (but never its parent directories).
|
||||
//
|
||||
// The size ceiling is enforced here, before the executor renders a Job, so an
|
||||
// oversized write is a clean 413 rather than an opaque rejection from the API
|
||||
// server when the Job spec breaches etcd's object limit. A body so large it also
|
||||
// breaches the shared 1 MiB envelope cap is refused earlier still, by decodeJSON,
|
||||
// as a 400 — the ceilings are layered, and the specific one answers first for
|
||||
// every plausible input.
|
||||
//
|
||||
// A write is audited; the two read operations are not, matching how the codebase
|
||||
// audits state changes (backup.create, image.admit) and not reads.
|
||||
func (a *API) handleWriteFile(w http.ResponseWriter, r *http.Request) {
|
||||
name, ok := a.authorizeFileOp(w, r)
|
||||
if !ok {
|
||||
return
|
||||
}
|
||||
path := r.URL.Query().Get("path")
|
||||
if path == "" {
|
||||
writeError(w, r, newError(http.StatusBadRequest, "bad_request",
|
||||
"the ?path= query parameter is required"))
|
||||
return
|
||||
}
|
||||
|
||||
var body writeFileRequest
|
||||
if err := decodeJSON(w, r, &body); err != nil {
|
||||
writeError(w, r, err)
|
||||
return
|
||||
}
|
||||
// decodeJSON enforces only DisallowUnknownFields, which rejects a MISSPELLED
|
||||
// field but not an omitted one — so presence is checked here, exactly as the
|
||||
// required ?path= is checked above and as docs/openapi.yaml already declares.
|
||||
if body.Content == nil {
|
||||
writeError(w, r, newError(http.StatusBadRequest, "bad_request",
|
||||
"the content field is required"))
|
||||
return
|
||||
}
|
||||
if len(*body.Content) > fileedit.MaxWriteBytes {
|
||||
writeError(w, r, newError(http.StatusRequestEntityTooLarge, "too_large",
|
||||
"file content is %d bytes; the editor writes at most %d",
|
||||
len(*body.Content), fileedit.MaxWriteBytes))
|
||||
return
|
||||
}
|
||||
|
||||
if err := a.Files.Write(r.Context(), name, path, *body.Content); err != nil {
|
||||
writeFileEditError(w, r, err)
|
||||
return
|
||||
}
|
||||
|
||||
p := principalFromContext(r.Context())
|
||||
a.audit(r, p.Email, "file.write", name+":"+path)
|
||||
writeJSON(w, http.StatusOK, map[string]any{"path": path, "status": "written"})
|
||||
}
|
||||
|
||||
// authorizeFileOp is the shared front half of all three file handlers — the gate
|
||||
// that decides whether this caller may touch this server's world at all. It
|
||||
// mirrors the backup/restore gate step for step, because it is guarding the same
|
||||
// resource under the same physical constraint:
|
||||
//
|
||||
// ① name validation — 400
|
||||
// ② ServerByName — an unknown server is 404
|
||||
// ③ owner-or-admin, else 403. An unowned (released) server fails for everyone
|
||||
// but admin, which is the same "must re-claim first" rule restore enforces
|
||||
// ④ stopped gate: the world PVC is RWO and held by a running server, so a file
|
||||
// Job cannot mount it — refuse unless the server is fully stopped. Ready means
|
||||
// it is up; any desiredState other than Stopped means it is up or coming up
|
||||
// and still owns the volume. This yields a specific 409 instead of a Job that
|
||||
// silently fails to mount
|
||||
// ⑤ the FileEditor must be wired, else 503
|
||||
//
|
||||
// Single-sourcing it is what keeps the three faces from drifting: a read path that
|
||||
// forgot the stopped gate would not merely fail, it would hang waiting for a Pod
|
||||
// that can never be scheduled.
|
||||
//
|
||||
// It returns the validated server name and false if it has already written a
|
||||
// response.
|
||||
func (a *API) authorizeFileOp(w http.ResponseWriter, r *http.Request) (string, bool) {
|
||||
name := r.PathValue("name")
|
||||
if err := naming.ValidateServerName(name); err != nil {
|
||||
writeError(w, r, newError(http.StatusBadRequest, "bad_name", "invalid server name: %v", err))
|
||||
return "", false
|
||||
}
|
||||
|
||||
p := principalFromContext(r.Context())
|
||||
rec, err := a.Repo.ServerByName(r.Context(), name)
|
||||
if err != nil {
|
||||
a.writeLookupError(w, r, err)
|
||||
return "", false
|
||||
}
|
||||
if !a.isOwnerOrAdmin(p, rec) {
|
||||
writeError(w, r, errForbidden)
|
||||
return "", false
|
||||
}
|
||||
|
||||
info, err := a.Cluster.GetServer(r.Context(), name)
|
||||
if err != nil {
|
||||
a.writeLookupError(w, r, err)
|
||||
return "", false
|
||||
}
|
||||
if info.Ready || info.DesiredState != string(v1alpha1.DesiredStopped) {
|
||||
writeError(w, r, newError(http.StatusConflict, "not_stopped",
|
||||
"stop the server before editing its files"))
|
||||
return "", false
|
||||
}
|
||||
|
||||
// Files is optional: when unwired the endpoints report 503 rather than
|
||||
// panicking, so the authorization boundary above is exercised even before the
|
||||
// file-Job executor is wired (see FileEditor).
|
||||
if a.Files == nil {
|
||||
writeError(w, r, newError(http.StatusServiceUnavailable, "files_unavailable",
|
||||
"the file editor is not configured"))
|
||||
return "", false
|
||||
}
|
||||
return name, true
|
||||
}
|
||||
|
||||
// writeFileEditError maps executor errors onto HTTP status codes. The three
|
||||
// sentinels are caller-fault and get precise answers; a timeout is reported as 504
|
||||
// so the caller knows to retry rather than believing the edit was rejected; and
|
||||
// anything else collapses to a 500 by writeError, so no cluster detail leaks.
|
||||
//
|
||||
// ErrBadPath is 400 rather than 403 on purpose: a path that escapes the world root
|
||||
// is a malformed request, not a permission the caller might be granted. Answering
|
||||
// 403 would imply some caller somewhere may read /etc/passwd through this endpoint,
|
||||
// and none may.
|
||||
func writeFileEditError(w http.ResponseWriter, r *http.Request, err error) {
|
||||
switch {
|
||||
case errors.Is(err, fileedit.ErrNotFound):
|
||||
writeError(w, r, newError(http.StatusNotFound, "not_found", "%s", err.Error()))
|
||||
case errors.Is(err, fileedit.ErrBadPath):
|
||||
writeError(w, r, newError(http.StatusBadRequest, "bad_path", "%s", err.Error()))
|
||||
case errors.Is(err, fileedit.ErrTooLarge):
|
||||
writeError(w, r, newError(http.StatusRequestEntityTooLarge, "too_large", "%s", err.Error()))
|
||||
case errors.Is(err, context.DeadlineExceeded):
|
||||
writeError(w, r, newError(http.StatusGatewayTimeout, "files_timeout",
|
||||
"the file operation did not finish in time; retry shortly"))
|
||||
default:
|
||||
writeError(w, r, err)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,447 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"felis.lolicon.best/internal/apis/felis/v1alpha1"
|
||||
"felis.lolicon.best/internal/fileedit"
|
||||
)
|
||||
|
||||
// fakeFileEditor records what the handlers ask the executor to do and returns
|
||||
// canned results. The real executor runs a Job and reads its log back; none of
|
||||
// that is the handlers' business, so the fake collapses it to "what was asked,
|
||||
// and what came back".
|
||||
type fakeFileEditor struct {
|
||||
err error
|
||||
|
||||
calls int
|
||||
gotServer string
|
||||
gotPath string
|
||||
gotContent []byte
|
||||
|
||||
entries []fileedit.Entry
|
||||
truncated bool
|
||||
content []byte
|
||||
}
|
||||
|
||||
func (f *fakeFileEditor) List(_ context.Context, server, path string) ([]fileedit.Entry, bool, error) {
|
||||
f.calls++
|
||||
f.gotServer, f.gotPath = server, path
|
||||
return f.entries, f.truncated, f.err
|
||||
}
|
||||
|
||||
func (f *fakeFileEditor) Read(_ context.Context, server, path string) ([]byte, error) {
|
||||
f.calls++
|
||||
f.gotServer, f.gotPath = server, path
|
||||
return f.content, f.err
|
||||
}
|
||||
|
||||
func (f *fakeFileEditor) Write(_ context.Context, server, path string, content []byte) error {
|
||||
f.calls++
|
||||
f.gotServer, f.gotPath, f.gotContent = server, path, content
|
||||
return f.err
|
||||
}
|
||||
|
||||
// mkFiles builds an API whose "survival" server is STOPPED and owned by owner1,
|
||||
// with a wired fakeFileEditor — the state in which every file operation is
|
||||
// permitted, so each subtest changes exactly the one thing it is about.
|
||||
func mkFiles() (*API, *fakeRepo, *fakeCluster, *fakeFileEditor) {
|
||||
repo := newFakeRepo()
|
||||
repo.byName["survival"] = &ServerRecord{Name: "survival", OwnerID: "owner1"}
|
||||
cl := newFakeCluster()
|
||||
cl.byName["survival"] = &ServerInfo{Name: "survival", Phase: "Stopped",
|
||||
Ready: false, DesiredState: string(v1alpha1.DesiredStopped)}
|
||||
files := &fakeFileEditor{}
|
||||
api := newTestAPI(repo, cl)
|
||||
api.Files = files
|
||||
return api, repo, cl, files
|
||||
}
|
||||
|
||||
// TestFileEditorStoppedGate is the gate this whole subsystem hinges on. The world
|
||||
// PVC is ReadWriteOnce, so a running server holds it and a file Job physically
|
||||
// cannot mount it — an ungated request would not fail cleanly, it would hang
|
||||
// waiting for a Pod that can never be scheduled. Every one of the three routes
|
||||
// must therefore refuse a non-stopped server with 409 not_stopped BEFORE reaching
|
||||
// the executor, which is why each asserts calls == 0 as well as the status.
|
||||
func TestFileEditorStoppedGate(t *testing.T) {
|
||||
owner := &Principal{UserID: "owner1", Email: "[email protected]", Role: "user"}
|
||||
|
||||
routes := []struct {
|
||||
name string
|
||||
method string
|
||||
path string
|
||||
body string
|
||||
}{
|
||||
{"list", "GET", "/api/v1/servers/survival/files?path=config", ""},
|
||||
{"read", "GET", "/api/v1/servers/survival/file?path=server.properties", ""},
|
||||
{"write", "PUT", "/api/v1/servers/survival/file?path=server.properties", `{"content":"aGk="}`},
|
||||
}
|
||||
|
||||
// Both non-stopped shapes matter and they are different states: a server that is
|
||||
// UP (Ready) plainly holds the volume, but so does one that is merely coming up
|
||||
// (desiredState=Running, not yet Ready) — the gate keys on intent as well as
|
||||
// readiness, exactly as the backup/restore gates do.
|
||||
states := []struct {
|
||||
name string
|
||||
ready bool
|
||||
desiredState v1alpha1.DesiredState
|
||||
}{
|
||||
{"running", true, v1alpha1.DesiredRunning},
|
||||
{"starting", false, v1alpha1.DesiredRunning},
|
||||
}
|
||||
|
||||
for _, rt := range routes {
|
||||
for _, st := range states {
|
||||
t.Run(fmt.Sprintf("%s on a %s server -> 409 not_stopped", rt.name, st.name), func(t *testing.T) {
|
||||
api, _, cl, files := mkFiles()
|
||||
cl.byName["survival"].Ready = st.ready
|
||||
cl.byName["survival"].DesiredState = string(st.desiredState)
|
||||
api.External = staticExternal{p: owner}
|
||||
|
||||
var hdr map[string]string
|
||||
if rt.body != "" {
|
||||
hdr = jsonHeader
|
||||
}
|
||||
w := do(api.ExternalHandler(), rt.method, rt.path, rt.body, hdr)
|
||||
if w.Code != http.StatusConflict || decodeErr(t, w) != "not_stopped" {
|
||||
t.Fatalf("code = %d body %s", w.Code, w.Body.String())
|
||||
}
|
||||
if files.calls != 0 {
|
||||
t.Fatal("a running server holds the RWO world PVC — the file Job must never be created")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestFileEditorAuthorization pins who may touch a world's files. It is the same
|
||||
// owner-or-admin rule the backup routes enforce, and it must hold on all three
|
||||
// routes — a read-only route leaking another owner's config (an RCON password
|
||||
// lives in server.properties) would be as bad as an unauthorized write.
|
||||
func TestFileEditorAuthorization(t *testing.T) {
|
||||
owner := &Principal{UserID: "owner1", Email: "[email protected]", Role: "user"}
|
||||
stranger := &Principal{UserID: "stranger", Email: "[email protected]", Role: "user"}
|
||||
admin := &Principal{UserID: "admin1", Email: "[email protected]", Role: "admin", ViaAdminAccess: true}
|
||||
|
||||
routes := []struct {
|
||||
name string
|
||||
method string
|
||||
path string
|
||||
body string
|
||||
}{
|
||||
{"list", "GET", "/api/v1/servers/survival/files", ""},
|
||||
{"read", "GET", "/api/v1/servers/survival/file?path=server.properties", ""},
|
||||
{"write", "PUT", "/api/v1/servers/survival/file?path=server.properties", `{"content":"aGk="}`},
|
||||
}
|
||||
|
||||
hdrFor := func(body string) map[string]string {
|
||||
if body != "" {
|
||||
return jsonHeader
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
for _, rt := range routes {
|
||||
t.Run(rt.name+": non-owner -> 403, executor untouched", func(t *testing.T) {
|
||||
api, _, _, files := mkFiles()
|
||||
api.External = staticExternal{p: stranger}
|
||||
w := do(api.ExternalHandler(), rt.method, rt.path, rt.body, hdrFor(rt.body))
|
||||
if w.Code != http.StatusForbidden {
|
||||
t.Fatalf("code = %d, want 403 (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
if files.calls != 0 {
|
||||
t.Fatal("a forbidden caller must not reach the file executor")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run(rt.name+": owner -> allowed", func(t *testing.T) {
|
||||
api, _, _, files := mkFiles()
|
||||
api.External = staticExternal{p: owner}
|
||||
w := do(api.ExternalHandler(), rt.method, rt.path, rt.body, hdrFor(rt.body))
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
if files.calls != 1 || files.gotServer != "survival" {
|
||||
t.Fatalf("executor saw (calls=%d, server=%q)", files.calls, files.gotServer)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run(rt.name+": admin on someone else's server -> allowed", func(t *testing.T) {
|
||||
api, repo, _, files := mkFiles()
|
||||
repo.byName["survival"].OwnerID = "someone-else"
|
||||
api.External = staticExternal{p: admin}
|
||||
w := do(api.ExternalHandler(), rt.method, rt.path, rt.body, hdrFor(rt.body))
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
if files.calls != 1 {
|
||||
t.Fatal("admin should reach the executor")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run(rt.name+": unowned server -> 403 for a plain user", func(t *testing.T) {
|
||||
api, repo, _, _ := mkFiles()
|
||||
repo.byName["survival"].OwnerID = "" // released world
|
||||
api.External = staticExternal{p: owner}
|
||||
w := do(api.ExternalHandler(), rt.method, rt.path, rt.body, hdrFor(rt.body))
|
||||
if w.Code != http.StatusForbidden {
|
||||
t.Fatalf("code = %d, want 403 (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run(rt.name+": unknown server -> 404", func(t *testing.T) {
|
||||
api, _, _, _ := mkFiles()
|
||||
api.External = staticExternal{p: owner}
|
||||
w := do(api.ExternalHandler(), rt.method,
|
||||
strings.Replace(rt.path, "survival", "missing", 1), rt.body, hdrFor(rt.body))
|
||||
if w.Code != http.StatusNotFound {
|
||||
t.Fatalf("code = %d, want 404 (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run(rt.name+": invalid server name -> 400 bad_name", func(t *testing.T) {
|
||||
api, _, _, _ := mkFiles()
|
||||
api.External = staticExternal{p: owner}
|
||||
w := do(api.ExternalHandler(), rt.method,
|
||||
strings.Replace(rt.path, "survival", "X", 1), rt.body, hdrFor(rt.body))
|
||||
if w.Code != http.StatusBadRequest || decodeErr(t, w) != "bad_name" {
|
||||
t.Fatalf("code = %d body %s", w.Code, w.Body.String())
|
||||
}
|
||||
})
|
||||
|
||||
t.Run(rt.name+": nil FileEditor -> 503 files_unavailable", func(t *testing.T) {
|
||||
api, _, _, _ := mkFiles()
|
||||
api.Files = nil
|
||||
api.External = staticExternal{p: owner}
|
||||
w := do(api.ExternalHandler(), rt.method, rt.path, rt.body, hdrFor(rt.body))
|
||||
if w.Code != http.StatusServiceUnavailable || decodeErr(t, w) != "files_unavailable" {
|
||||
t.Fatalf("code = %d body %s", w.Code, w.Body.String())
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestFileEditorHandlers covers the per-route behaviour the shared gate does not:
|
||||
// what is passed through to the executor and what comes back.
|
||||
func TestFileEditorHandlers(t *testing.T) {
|
||||
owner := &Principal{UserID: "owner1", Email: "[email protected]", Role: "user"}
|
||||
|
||||
t.Run("list passes the path through and returns entries", func(t *testing.T) {
|
||||
api, _, _, files := mkFiles()
|
||||
files.entries = []fileedit.Entry{{Name: "paper.yml", Size: 12}, {Name: "sub", IsDir: true}}
|
||||
files.truncated = true
|
||||
api.External = staticExternal{p: owner}
|
||||
|
||||
w := do(api.ExternalHandler(), "GET", "/api/v1/servers/survival/files?path=config", "", nil)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("code = %d (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
var resp struct {
|
||||
Path string `json:"path"`
|
||||
Entries []fileedit.Entry `json:"entries"`
|
||||
Truncated bool `json:"truncated"`
|
||||
}
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("body not JSON: %v (%s)", err, w.Body.String())
|
||||
}
|
||||
if files.gotPath != "config" {
|
||||
t.Fatalf("executor saw path %q, want the query value verbatim", files.gotPath)
|
||||
}
|
||||
if resp.Path != "config" || len(resp.Entries) != 2 || !resp.Truncated {
|
||||
t.Fatalf("unexpected response %+v", resp)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("list without a path lists the world root", func(t *testing.T) {
|
||||
api, _, _, files := mkFiles()
|
||||
api.External = staticExternal{p: owner}
|
||||
w := do(api.ExternalHandler(), "GET", "/api/v1/servers/survival/files", "", nil)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("code = %d (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
if files.gotPath != "" {
|
||||
t.Fatalf("path = %q, want empty (the root)", files.gotPath)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("read returns base64 content", func(t *testing.T) {
|
||||
api, _, _, files := mkFiles()
|
||||
files.content = []byte("motd=hello\n")
|
||||
api.External = staticExternal{p: owner}
|
||||
|
||||
w := do(api.ExternalHandler(), "GET", "/api/v1/servers/survival/file?path=server.properties", "", nil)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("code = %d (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
var resp struct {
|
||||
Path string `json:"path"`
|
||||
Content []byte `json:"content"`
|
||||
}
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil {
|
||||
t.Fatalf("body not JSON: %v", err)
|
||||
}
|
||||
if resp.Path != "server.properties" || string(resp.Content) != "motd=hello\n" {
|
||||
t.Fatalf("unexpected response %+v (%q)", resp, resp.Content)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("read without a path -> 400", func(t *testing.T) {
|
||||
api, _, _, files := mkFiles()
|
||||
api.External = staticExternal{p: owner}
|
||||
w := do(api.ExternalHandler(), "GET", "/api/v1/servers/survival/file", "", nil)
|
||||
if w.Code != http.StatusBadRequest {
|
||||
t.Fatalf("code = %d, want 400", w.Code)
|
||||
}
|
||||
if files.calls != 0 {
|
||||
t.Fatal("a pathless read must not reach the executor")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("write decodes content, audits, and answers 200", func(t *testing.T) {
|
||||
api, repo, _, files := mkFiles()
|
||||
api.External = staticExternal{p: owner}
|
||||
|
||||
w := do(api.ExternalHandler(), "PUT", "/api/v1/servers/survival/file?path=server.properties",
|
||||
`{"content":"bW90ZD1jaGFuZ2VkCg=="}`, jsonHeader)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("code = %d (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
if string(files.gotContent) != "motd=changed\n" {
|
||||
t.Fatalf("executor got content %q, want the decoded bytes", files.gotContent)
|
||||
}
|
||||
if len(repo.audits) != 1 || repo.audits[0].Action != "file.write" ||
|
||||
repo.audits[0].Actor != "[email protected]" {
|
||||
t.Fatalf("write not audited as expected: %+v", repo.audits)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("reads are not audited", func(t *testing.T) {
|
||||
api, repo, _, _ := mkFiles()
|
||||
api.External = staticExternal{p: owner}
|
||||
do(api.ExternalHandler(), "GET", "/api/v1/servers/survival/files", "", nil)
|
||||
do(api.ExternalHandler(), "GET", "/api/v1/servers/survival/file?path=x", "", nil)
|
||||
if len(repo.audits) != 0 {
|
||||
t.Fatalf("reads should not write audit rows: %+v", repo.audits)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("oversized write -> 413 before the executor", func(t *testing.T) {
|
||||
api, _, _, files := mkFiles()
|
||||
api.External = staticExternal{p: owner}
|
||||
// base64 of MaxWriteBytes+1 zero bytes, built as a JSON body.
|
||||
body, err := json.Marshal(writeFileRequest{Content: bytesPtr(make([]byte, fileedit.MaxWriteBytes+1))})
|
||||
if err != nil {
|
||||
t.Fatalf("marshal: %v", err)
|
||||
}
|
||||
w := do(api.ExternalHandler(), "PUT", "/api/v1/servers/survival/file?path=big.txt",
|
||||
string(body), jsonHeader)
|
||||
if w.Code != http.StatusRequestEntityTooLarge || decodeErr(t, w) != "too_large" {
|
||||
t.Fatalf("code = %d body %s", w.Code, w.Body.String())
|
||||
}
|
||||
if files.calls != 0 {
|
||||
t.Fatal("an oversized write must be refused before a Job is rendered")
|
||||
}
|
||||
})
|
||||
|
||||
// A body that omits "content" must be refused, not treated as empty. Before
|
||||
// Content became a *[]byte, {} decoded to nil and travelled all the way to
|
||||
// O_TRUNC — so a client serialisation bug answered 200 while zeroing the very
|
||||
// config the caller opened the editor to repair. An explicit "" stays legal,
|
||||
// because a deliberate truncate is a real edit; only the OMISSION is refused.
|
||||
t.Run("a write with no content field -> 400, never a truncate", func(t *testing.T) {
|
||||
for _, body := range []string{`{}`, `{"content":null}`} {
|
||||
api, _, _, files := mkFiles()
|
||||
api.External = staticExternal{p: owner}
|
||||
w := do(api.ExternalHandler(), "PUT", "/api/v1/servers/survival/file?path=server.properties",
|
||||
body, jsonHeader)
|
||||
if w.Code != http.StatusBadRequest || decodeErr(t, w) != "bad_request" {
|
||||
t.Fatalf("body %s: code = %d, want 400 bad_request (%s)", body, w.Code, w.Body.String())
|
||||
}
|
||||
if files.calls != 0 {
|
||||
t.Fatalf("body %s: reached the executor; an omitted content field must never truncate", body)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("an explicit empty content is a legitimate truncate", func(t *testing.T) {
|
||||
api, _, _, files := mkFiles()
|
||||
api.External = staticExternal{p: owner}
|
||||
w := do(api.ExternalHandler(), "PUT", "/api/v1/servers/survival/file?path=server.properties",
|
||||
`{"content":""}`, jsonHeader)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
if files.calls != 1 || len(files.gotContent) != 0 {
|
||||
t.Fatalf("calls = %d, content = %d bytes; want one call writing 0 bytes",
|
||||
files.calls, len(files.gotContent))
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("a write at exactly the limit is allowed", func(t *testing.T) {
|
||||
api, _, _, files := mkFiles()
|
||||
api.External = staticExternal{p: owner}
|
||||
body, err := json.Marshal(writeFileRequest{Content: bytesPtr(make([]byte, fileedit.MaxWriteBytes))})
|
||||
if err != nil {
|
||||
t.Fatalf("marshal: %v", err)
|
||||
}
|
||||
w := do(api.ExternalHandler(), "PUT", "/api/v1/servers/survival/file?path=big.txt",
|
||||
string(body), jsonHeader)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("code = %d, want 200 — the limit is inclusive (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
if len(files.gotContent) != fileedit.MaxWriteBytes {
|
||||
t.Fatalf("executor got %d bytes, want %d", len(files.gotContent), fileedit.MaxWriteBytes)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// TestFileEditorErrorMapping proves each executor sentinel reaches the caller as the
|
||||
// right status. The containment refusal mapping to 400 (not 403) is the one worth
|
||||
// stating: an escaping path is a malformed request, not a permission a caller might
|
||||
// be granted.
|
||||
func TestFileEditorErrorMapping(t *testing.T) {
|
||||
owner := &Principal{UserID: "owner1", Email: "[email protected]", Role: "user"}
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
err error
|
||||
wantCode int
|
||||
wantErr string
|
||||
}{
|
||||
{"escaping path", fmt.Errorf("%w: nope", fileedit.ErrBadPath), http.StatusBadRequest, "bad_path"},
|
||||
{"missing file", fmt.Errorf("%w: nope", fileedit.ErrNotFound), http.StatusNotFound, "not_found"},
|
||||
{"oversized file", fmt.Errorf("%w: nope", fileedit.ErrTooLarge), http.StatusRequestEntityTooLarge, "too_large"},
|
||||
{"timeout", fmt.Errorf("waiting: %w", context.DeadlineExceeded), http.StatusGatewayTimeout, "files_timeout"},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
api, _, _, files := mkFiles()
|
||||
files.err = tc.err
|
||||
api.External = staticExternal{p: owner}
|
||||
w := do(api.ExternalHandler(), "GET", "/api/v1/servers/survival/file?path=x", "", nil)
|
||||
if w.Code != tc.wantCode || decodeErr(t, w) != tc.wantErr {
|
||||
t.Fatalf("code = %d body %s, want %d/%s", w.Code, w.Body.String(), tc.wantCode, tc.wantErr)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
t.Run("an unrecognised executor failure -> 500", func(t *testing.T) {
|
||||
api, _, _, files := mkFiles()
|
||||
files.err = fmt.Errorf("the job pod exploded")
|
||||
api.External = staticExternal{p: owner}
|
||||
w := do(api.ExternalHandler(), "GET", "/api/v1/servers/survival/file?path=x", "", nil)
|
||||
if w.Code != http.StatusInternalServerError {
|
||||
t.Fatalf("code = %d, want 500 (%s)", w.Code, w.Body.String())
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// bytesPtr builds the *[]byte writeFileRequest.Content wants. The pointer is what
|
||||
// lets an omitted field be distinguished from an empty one; see the type's comment.
|
||||
func bytesPtr(b []byte) *[]byte { return &b }
|
||||
Reference in new issue
Block a user