diff --git a/cmd/felis/api.go b/cmd/felis/api.go index 23d235d..1c5623b 100644 --- a/cmd/felis/api.go +++ b/cmd/felis/api.go @@ -16,6 +16,7 @@ import ( "felis.lolicon.best/internal/backupjob" "felis.lolicon.best/internal/build" "felis.lolicon.best/internal/config" + "felis.lolicon.best/internal/fileedit" "felis.lolicon.best/internal/mail" "felis.lolicon.best/internal/panel" "felis.lolicon.best/internal/passkey" @@ -222,6 +223,24 @@ func cmdAPI(args []string, stdout, stderr io.Writer) int { fmt.Fprintln(stderr, "felis api: backup executor disabled (needs FELIS_IMAGE and FELIS_BACKUP_PVC) — backup endpoint returns 503") } + // Server file editor: a weak-SA Job mounts ONLY the target world PVC and runs + // `felis files`, printing its result for felis-api to read back through + // pods/log (see internal/fileedit). It needs FELIS_IMAGE but — unlike restore + // and backup — no backup PVC, since it never touches the archive store, so it + // is wired on the image alone; otherwise the editor is left nil and the file + // endpoints honestly return 503. It takes the typed clientset rather than the + // controller-runtime client because the log subresource lives only on the typed + // CoreV1 client, and one client covers its Job create, Pod list, and log read. + var files api.FileEditor + if felisImage != "" { + files = &fileedit.Editor{ + Runner: fileedit.NewK8sRunner(clientset), + Config: fileEditConfig(cfg, felisImage), + } + } else { + fmt.Fprintln(stderr, "felis api: file editor disabled (needs FELIS_IMAGE) — file endpoints return 503") + } + // One PGRepo instance backs both the handlers and the session verifier: the // SessionAuth that fronts the external face reads sessions/users/settings from // the same store the auth handlers write to, so a login and the next request @@ -240,6 +259,7 @@ func cmdAPI(args []string, stdout, stderr io.Writer) int { Builder: builder, Restorer: restorer, Backuper: backuper, + Files: files, Submissions: submissions, Mailer: mailer, // The external face is fronted by SessionAuth: it prefers a local session @@ -458,6 +478,19 @@ func backupConfig(cfg *config.Config, image, backupPVC string) backupjob.Config } } +// fileEditConfig builds the file editor's config from felis.toml plus the +// deployment-supplied image. It is the shortest of the three: the editor mounts +// only the world PVC, so it needs no archive coordinates at all, and everything +// else — the weak SA, the "/data" world root that makes paths match what the +// minecraft server itself sees, the runtime identity, and the size/time ceilings — +// falls back to the fileedit package's hardened defaults. +func fileEditConfig(cfg *config.Config, image string) fileedit.Config { + return fileedit.Config{ + Namespace: cfg.K8s.Namespace, + Image: image, + } +} + // reconcileBuilds polls unfinished builds on an interval and advances any whose // Job has reached a terminal phase. It exits when ctx is cancelled. func reconcileBuilds(ctx context.Context, b *build.Builder, stderr io.Writer) { diff --git a/cmd/felis/files.go b/cmd/felis/files.go new file mode 100644 index 0000000..49fd2dc --- /dev/null +++ b/cmd/felis/files.go @@ -0,0 +1,84 @@ +package main + +import ( + "encoding/base64" + "flag" + "fmt" + "io" + "os" + + "felis.lolicon.best/internal/fileedit" +) + +// cmdFiles is the in-Pod entrypoint the file-editor Job runs. internal/fileedit +// renders a Pod whose command is `/usr/local/bin/felis files`. It performs ONE +// file operation against the mounted world volume, prints the result as a single +// marked JSON line on stdout, and exits — it is NOT a user-facing command and is +// never invoked by hand. +// +// Like cmdRestore it deliberately holds NO database credentials and never calls +// config.Load: felis-api made the authorization decision (the caller owns this +// server, and the server is stopped so the RWO world volume is free); this process +// is the unprivileged hands that touch bytes. Its entire input is the three flags +// below plus, for a write, one environment variable. Every isolation guarantee +// lives in the Pod spec (internal/fileedit/jobspec.go), and the path-containment +// guarantee lives in fileedit.Execute, which resolves the path through os.Root and +// therefore cannot be walked out of the world mount. +// +// Exit status carries a specific meaning that felis-api depends on: a CALLER-fault +// outcome — a path that escapes the root, a file that is missing or too large — is +// a SUCCESSFUL run that prints a Result carrying an error code, so the API can map +// it to a precise 4xx. A non-zero exit means the operation could not be attempted +// at all (the world mount is unreadable, the result unprintable), which the API +// reports as a 500. +func cmdFiles(args []string, stdout, stderr io.Writer) int { + fs := flag.NewFlagSet("files", flag.ContinueOnError) + fs.SetOutput(stderr) + op := fs.String("op", "", "operation: list, read, or write") + path := fs.String("path", "", "path to operate on, relative to the world root (empty = the root itself)") + worldsRoot := fs.String("worlds-root", "/data", "mount path of the world PVC; every path resolves under it") + if err := fs.Parse(args); err != nil { + return 2 + } + + if *op == "" { + fmt.Fprintln(stderr, "felis files: --op is required (list, read, or write)") + return 2 + } + + // New content arrives base64-encoded in the environment rather than in argv: + // a process's arguments are world-readable on the node (/proc//cmdline), + // whereas its environment is not, and a config file being written can carry + // secrets — an RCON password in server.properties is the obvious case. The + // encoding is what lets arbitrary bytes (CRLF endings, a BOM, a NUL) survive a + // channel that must be a valid string. + var content []byte + if *op == fileedit.OpWrite { + raw, ok := os.LookupEnv(fileedit.ContentEnv) + if !ok { + fmt.Fprintf(stderr, "felis files: a write needs %s in the environment\n", fileedit.ContentEnv) + return 2 + } + decoded, err := base64.StdEncoding.DecodeString(raw) + if err != nil { + fmt.Fprintf(stderr, "felis files: %s is not valid base64: %v\n", fileedit.ContentEnv, err) + return 2 + } + content = decoded + } + + res, err := fileedit.Execute(*worldsRoot, *op, *path, content) + if err != nil { + // The operation could not be attempted — infrastructure, not caller fault. + fmt.Fprintf(stderr, "felis files: %v\n", err) + return 1 + } + if err := fileedit.Print(stdout, res); err != nil { + // The result exists but could not be delivered. Exiting non-zero is the only + // honest signal left: felis-api would otherwise find no marked line and have + // to guess why. + fmt.Fprintf(stderr, "felis files: %v\n", err) + return 1 + } + return 0 +} diff --git a/cmd/felis/run.go b/cmd/felis/run.go index 0054507..9652632 100644 --- a/cmd/felis/run.go +++ b/cmd/felis/run.go @@ -18,6 +18,7 @@ Commands: reaper Run the world reaper / backup batch restore Extract a world archive into a world volume (internal Job entrypoint) backup Archive a world into the backup store and record it (internal Job entrypoint) + files List/read/write one file in a stopped server's world (internal Job entrypoint) manifests Render the control-plane RBAC + NetworkPolicy install bundle as YAML apply Create a MinecraftServer CRD (direct K8s write; use -f server.json) setup Run host bootstrap + first-run setup console (TUI; requires root/sudo) @@ -45,6 +46,7 @@ var commands = map[string]func(args []string, stdout, stderr io.Writer) int{ "reaper": cmdReaper, "restore": cmdRestore, "backup": cmdBackup, + "files": cmdFiles, "manifests": cmdManifests, "apply": cmdApply, "setup": cmdSetup, diff --git a/docs/openapi.yaml b/docs/openapi.yaml index d42e37a..6618313 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -2771,6 +2771,215 @@ paths: '503': $ref: '#/components/responses/ServiceUnavailable' + # ------------------------------------------------- server file editor (app) --- + /api/v1/servers/{name}/files: + get: + tags: [files] + operationId: listServerFiles + summary: List a directory in a server's world volume (owner-or-admin; server must be stopped). + description: >- + Lists one directory inside the server's world volume — the repair lever for a + server that will not boot because a config file is wrong. The world PVC is RWO + and held by a running server, so the server must be fully stopped first (409 + not_stopped otherwise). The listing runs as a one-shot Job whose output is read + back through pods/log, so the call is synchronous but takes seconds rather than + milliseconds. Paths are resolved inside the world root by os.Root, so "..", an + absolute path, and a symlink leaving the root are all refused with 400 bad_path. + Listings are capped; truncated reports that the cap was hit. + x-felis-face: [external] + x-felis-tier: app + security: [{ accessJWT: [] }] + parameters: + - { name: name, in: path, required: true, schema: { type: string } } + - name: path + in: query + required: false + description: Directory to list, relative to the world root. Empty lists the root itself. + schema: { type: string } + responses: + '200': + description: Directory listing. + content: + application/json: + schema: + type: object + required: [path, entries, truncated] + properties: + path: { type: string } + truncated: { type: boolean, description: The listing hit the entry cap and is incomplete. } + entries: + type: array + items: + type: object + required: [name, size, is_dir, mod_time] + properties: + name: { type: string } + size: { type: integer, format: int64 } + is_dir: { type: boolean } + mod_time: { type: string, format: date-time } + '400': + description: Invalid server name, or a path that escapes the world root. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + '404': + description: Unknown server, or no such directory. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + '409': + description: Server is not stopped (its world PVC is still mounted). + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + '503': + $ref: '#/components/responses/ServiceUnavailable' + '504': + description: The file Job did not finish in time; retry. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + + /api/v1/servers/{name}/file: + get: + tags: [files] + operationId: readServerFile + summary: Read a file from a server's world volume (owner-or-admin; server must be stopped). + description: >- + Returns one file's bytes, base64-encoded, from inside the server's world + volume. Same stopped-gate and os.Root containment as the directory listing. + Reads are capped at 1 MiB; a larger file is 413 rather than a truncated read, + because a config editor that silently returned half a file would let a + subsequent save destroy the other half. + x-felis-face: [external] + x-felis-tier: app + security: [{ accessJWT: [] }] + parameters: + - { name: name, in: path, required: true, schema: { type: string } } + - name: path + in: query + required: true + description: File to read, relative to the world root. + schema: { type: string } + responses: + '200': + description: File contents. + content: + application/json: + schema: + type: object + required: [path, content] + properties: + path: { type: string } + content: { type: string, format: byte, description: Base64-encoded file bytes. } + '400': + description: Missing path, invalid server name, or a path that escapes the world root. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + '404': + description: Unknown server, or no such file. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + '409': + description: Server is not stopped (its world PVC is still mounted). + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + '413': + description: The file is larger than the editor reads. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + '503': + $ref: '#/components/responses/ServiceUnavailable' + '504': + description: The file Job did not finish in time; retry. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + put: + tags: [files] + operationId: writeServerFile + summary: Write a file in a server's world volume (owner-or-admin; server must be stopped). + description: >- + Replaces a file's contents, creating the file if absent but never creating its + parent directories. Content is base64 so arbitrary bytes (CRLF endings, a BOM) + survive intact. Writes are capped at 256 KiB — the Job spec carries the content, + and etcd bounds the object — so a larger body is 413. Same stopped-gate and + os.Root containment as the read; a write through a symlink leaving the world + root is refused. Audited as file.write. + x-felis-face: [external] + x-felis-tier: app + security: [{ accessJWT: [] }] + parameters: + - { name: name, in: path, required: true, schema: { type: string } } + - name: path + in: query + required: true + description: File to write, relative to the world root. + schema: { type: string } + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [content] + properties: + content: { type: string, format: byte, description: Base64-encoded file bytes. } + responses: + '200': + description: File written. + content: + application/json: + schema: + type: object + required: [path, status] + properties: + path: { type: string } + status: { type: string, const: written } + '400': + description: Missing path, malformed body, invalid server name, or a path that escapes the world root. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + '404': + description: Unknown server, or the parent directory does not exist. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + '409': + description: Server is not stopped (its world PVC is still mounted). + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + '413': + description: The content is larger than the editor writes. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + '503': + $ref: '#/components/responses/ServiceUnavailable' + '504': + description: The file Job did not finish in time; retry. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + # ------------------------------------------------------ users (admin tier) ---- /api/v1/users: get: diff --git a/internal/api/api.go b/internal/api/api.go index f97a5a2..db2f6bf 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -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 diff --git a/internal/api/handlers_files.go b/internal/api/handlers_files.go new file mode 100644 index 0000000..1ed6770 --- /dev/null +++ b/internal/api/handlers_files.go @@ -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) + } +} diff --git a/internal/api/handlers_files_test.go b/internal/api/handlers_files_test.go new file mode 100644 index 0000000..0d82900 --- /dev/null +++ b/internal/api/handlers_files_test.go @@ -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: "owner1@example.net", 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: "owner1@example.net", Role: "user"} + stranger := &Principal{UserID: "stranger", Email: "stranger@example.net", Role: "user"} + admin := &Principal{UserID: "admin1", Email: "admin1@example.net", 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: "owner1@example.net", 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 != "owner1@example.net" { + 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: "owner1@example.net", 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 } diff --git a/internal/fileedit/editor.go b/internal/fileedit/editor.go new file mode 100644 index 0000000..d9061f5 --- /dev/null +++ b/internal/fileedit/editor.go @@ -0,0 +1,311 @@ +// Package fileedit implements the server file editor (list / read / write a file +// in a server's world volume), the lever an owner reaches for when a server will +// not boot because one line of server.properties or a plugin's YAML is wrong — +// the one repair that otherwise requires a human with cluster access. +// +// felis-api cannot touch a world in-process: the world PVC is ReadWriteOnce and +// its lifecycle is owned by the operator's StatefulSet, so the API has nothing to +// mount at request time. That is the same constraint that makes internal/restore +// and internal/backupjob one-shot Jobs, and it has the same two consequences here: +// the work runs as a Job, and the server MUST be stopped first (a running server +// holds the RWO volume, so the Job could not mount it). The handlers enforce the +// stopped gate exactly as the backup/restore handlers do. +// +// # How a result gets back +// +// A file operation is unusual among Felis's Jobs in that the CALLER wants the +// output, not just the side effect: a listing and a file's bytes must reach the +// browser that asked. The transport is deliberately the narrowest one available — +// the Job PRINTS its result to stdout and felis-api reads it back through the +// pods/log subresource, which it already has RBAC for. This is the whole reason +// the design needs no new permission: +// +// create the Job → jobs:create (already held) +// find its Pod → pods:list (already held, for the §8 console) +// read the result → pods/log:get (already held, for the §8 console) +// +// No pods/exec, no pods/portforward, not even pods:get — the least-privilege line +// internal/platform/rbac.go draws and a test asserts. The write direction travels +// the other way, on the Job spec felis-api creates (see ContentEnv). +// +// The price is latency: every operation is a Pod schedule + image pull, so a +// listing takes seconds rather than milliseconds. That is inherent to RWO plus a +// stopped server, not a property of this transport, and it is why the editor is a +// repair tool rather than a file manager. +// +// The Editor depends on the Runner interface, so the orchestration and the error +// mapping are unit-tested against an in-memory fake; the client-go implementation +// (k8sjobs.go) compiles here but is exercised only against a live cluster. +package fileedit + +import ( + "context" + "crypto/rand" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "time" + + "felis.lolicon.best/internal/naming" +) + +// Errors the Editor returns, which internal/api maps onto HTTP status codes +// (handlers_files.go). They are sentinels rather than an error type because the +// mapping needs nothing but identity — the human-readable detail rides along in +// the wrapped message. +var ( + // ErrNotFound is a path that resolves inside the world root but has nothing at + // it. It is distinct from a missing SERVER, which the handler resolves earlier. + ErrNotFound = errors.New("fileedit: no such file or directory") + // ErrBadPath is a path the Job refused: it escapes the world root (via "..", an + // absolute path, or a symlink), or names a directory where a file is required. + ErrBadPath = errors.New("fileedit: path is not accessible") + // ErrTooLarge is a read of a file over MaxReadBytes or a write over + // MaxWriteBytes. + ErrTooLarge = errors.New("fileedit: file is too large for the editor") +) + +// Runner is the cluster-side half of one file operation: render and create the +// Job, wait for its Pod to reach a terminal phase, and return the marked JSON +// payload the Pod printed. It is one method rather than a create/poll/read trio +// because felis-api cannot poll a Job at all (no jobs:get — see FilesJobName), so +// there is no intermediate state a caller could usefully observe; the operation is +// synchronous from the API's point of view whether or not the seam pretends +// otherwise. +// +// It is an interface so the Editor's orchestration and error mapping are tested +// against a fake; the client-go implementation (K8sRunner) is integration-only. +type Runner interface { + // Run creates the Job for p and returns the raw JSON payload from the + // ResultPrefix line of its Pod's log. + Run(ctx context.Context, p JobParams) ([]byte, error) +} + +// Config parameterises the file editor. Image has no default on purpose: it is +// deployment-specific, and when it is empty cmd/felis leaves the API's FileEditor +// nil so the endpoints report 503 rather than creating a Job that cannot run. +type Config struct { + // Namespace is where the world PVCs live and the Job runs (the minecraft + // namespace), co-located with the world it edits. + Namespace string + // ServiceAccount is the weak SA the Pod runs as. It reuses felis-restore (bare, + // no Role/RoleBinding anywhere): a file-editor Pod needs no K8s API access, only + // filesystem access to the one PVC it mounts, so a second identity with the same + // empty powers would be a manifest to maintain for no isolation gain. + ServiceAccount string + // Image is the felis binary image; the Job runs `felis files` from it. + Image string + // WorldsRoot is the in-Pod mount path of the world PVC, and therefore the root + // every caller-supplied path is resolved against. It defaults to the operator's + // own dataMountPath ("/data") rather than restore's "/world" so the paths a user + // types are the paths the MINECRAFT SERVER sees: "server.properties" means the + // same file in the editor as it does in every wiki page and support thread. + WorldsRoot string + // Deadline caps the Pod's wall-clock (activeDeadlineSeconds). + Deadline time.Duration + // Timeout caps how long felis-api waits for a result before giving up. It bounds + // an HTTP handler's block, so it is the tighter of the two: a Pod that is still + // pulling its image when this expires leaves the caller with a clean 504 while + // the Job runs on harmlessly to its own Deadline and is then TTL'd away. + Timeout time.Duration + // CPULimit / MemLimit cap the container. + CPULimit string + MemLimit string + // RunAsUser / RunAsGroup / FSGroup are the Pod's runtime identity. FSGroup MUST + // match the operator StatefulSet's runtime group, or a file this Pod writes + // would be unreadable by the minecraft server that later mounts the same PVC. + RunAsUser int64 + RunAsGroup int64 + FSGroup int64 + // TTLAfterFinished is how long a finished Job lingers before the Job controller + // collects it. felis-api holds no jobs:delete, so this is the ONLY cleanup path; + // it must stay comfortably longer than the moment felis-api needs to read the + // Pod's log, because the TTL takes the Pod (and its log) with the Job. + TTLAfterFinished time.Duration +} + +// defaults applied when a Config field is left zero. They are sized for what a +// file operation actually is — open one file, print a few KiB — which is orders of +// magnitude smaller than a restore's tar of an entire world. +const ( + defaultNamespace = "minecraft" + defaultServiceAccount = "felis-restore" + defaultWorldsRoot = "/data" + defaultDeadline = 2 * time.Minute + defaultTimeout = 90 * time.Second + defaultCPULimit = "500m" + defaultMemLimit = "256Mi" + defaultRunAsID = int64(1000) + defaultTTL = 2 * time.Minute +) + +// withDefaults returns a copy of c with zero fields filled, so a partially +// configured Config (or the zero value, in tests) is always usable. +func (c Config) withDefaults() Config { + if c.Namespace == "" { + c.Namespace = defaultNamespace + } + if c.ServiceAccount == "" { + c.ServiceAccount = defaultServiceAccount + } + if c.WorldsRoot == "" { + c.WorldsRoot = defaultWorldsRoot + } + if c.Deadline <= 0 { + c.Deadline = defaultDeadline + } + if c.Timeout <= 0 { + c.Timeout = defaultTimeout + } + if c.CPULimit == "" { + c.CPULimit = defaultCPULimit + } + if c.MemLimit == "" { + c.MemLimit = defaultMemLimit + } + if c.RunAsUser == 0 { + c.RunAsUser = defaultRunAsID + } + if c.RunAsGroup == 0 { + c.RunAsGroup = defaultRunAsID + } + if c.FSGroup == 0 { + c.FSGroup = defaultRunAsID + } + if c.TTLAfterFinished <= 0 { + c.TTLAfterFinished = defaultTTL + } + return c +} + +// Editor is the production internal/api.FileEditor. It holds no mutable state. +type Editor struct { + Runner Runner + Config Config +} + +// List returns one directory's entries, resolved under the server's world root. +// An empty path lists the world root itself. +func (e *Editor) List(ctx context.Context, server, path string) ([]Entry, bool, error) { + res, err := e.run(ctx, server, OpList, path, nil) + if err != nil { + return nil, false, err + } + // A genuinely empty directory unmarshals Entries as nil; normalise it so the + // handler serialises [] rather than null. + if res.Entries == nil { + res.Entries = []Entry{} + } + return res.Entries, res.Truncated, nil +} + +// Read returns a file's bytes, resolved under the server's world root. +func (e *Editor) Read(ctx context.Context, server, path string) ([]byte, error) { + res, err := e.run(ctx, server, OpRead, path, nil) + if err != nil { + return nil, err + } + // A zero-length file unmarshals Content as nil, which is a legitimate result, + // not an error — normalise so the caller never has to distinguish nil from empty. + if res.Content == nil { + res.Content = []byte{} + } + return res.Content, nil +} + +// Write replaces a file's contents, creating it if absent (but never creating +// parent directories — see the write helper in exec.go). +func (e *Editor) Write(ctx context.Context, server, path string, content []byte) error { + _, err := e.run(ctx, server, OpWrite, path, content) + return err +} + +// run is the shared body of all three operations: mint an op id, render the +// params, run the Job, and translate the Result's code into a sentinel error. +// +// The size check happens HERE, before a Job is created, as well as inside the Pod. +// That is not redundancy for its own sake: an oversized write would otherwise be +// rejected by the API SERVER (etcd's object limit) as an opaque failure, long after +// felis-api had committed to the request, instead of as a clean 413. +func (e *Editor) run(ctx context.Context, server, op, path string, content []byte) (Result, error) { + if op == OpWrite && len(content) > MaxWriteBytes { + return Result{}, fmt.Errorf("%w: content is %d bytes, the limit is %d", + ErrTooLarge, len(content), MaxWriteBytes) + } + + cfg := e.Config.withDefaults() + opID, err := newOpID() + if err != nil { + return Result{}, err + } + + // Bound the wait here rather than trusting the caller's context: this is an HTTP + // handler's goroutine and the Pod it waits on may never become ready (an + // unschedulable node, an unpullable image). The Job's own activeDeadlineSeconds + // cleans up the cluster side independently. + ctx, cancel := context.WithTimeout(ctx, cfg.Timeout) + defer cancel() + + payload, err := e.Runner.Run(ctx, JobParams{ + Server: server, + OpID: opID, + Op: op, + Path: path, + Content: content, + WorldPVC: naming.WorldPVCName(server), + Namespace: cfg.Namespace, + ServiceAccount: cfg.ServiceAccount, + Image: cfg.Image, + WorldsRoot: cfg.WorldsRoot, + Deadline: cfg.Deadline, + CPULimit: cfg.CPULimit, + MemLimit: cfg.MemLimit, + RunAsUser: cfg.RunAsUser, + RunAsGroup: cfg.RunAsGroup, + FSGroup: cfg.FSGroup, + TTLAfterFinished: cfg.TTLAfterFinished, + }) + if err != nil { + return Result{}, err + } + + var res Result + if err := json.Unmarshal(payload, &res); err != nil { + return Result{}, fmt.Errorf("fileedit: malformed result from the file Job: %w", err) + } + return res, resultError(res) +} + +// resultError translates a Result's code into the sentinel the API maps. An +// unrecognised code is deliberately NOT swallowed as success: a Job reporting a +// failure this build does not know about must still fail the request, or a future +// code would silently read as "it worked". +func resultError(res Result) error { + switch res.Code { + case "": + return nil + case CodeNotFound: + return fmt.Errorf("%w: %s", ErrNotFound, res.Error) + case CodeBadPath: + return fmt.Errorf("%w: %s", ErrBadPath, res.Error) + case CodeTooLarge: + return fmt.Errorf("%w: %s", ErrTooLarge, res.Error) + default: + return fmt.Errorf("fileedit: file operation failed (%s): %s", res.Code, res.Error) + } +} + +// newOpID mints the per-invocation tag that names the Job and labels its Pod. 64 +// bits of randomness is far more than collision-avoidance needs (a collision only +// matters between two operations alive in the same TTL window), but the id is also +// what selects THIS operation's Pod when reading the result back — so a collision +// would mean reading another operation's output, and the margin is cheap. Hex +// keeps it a valid DNS-1123 name fragment and a valid label value. +func newOpID() (string, error) { + var b [8]byte + if _, err := rand.Read(b[:]); err != nil { + return "", fmt.Errorf("fileedit: generate op id: %w", err) + } + return hex.EncodeToString(b[:]), nil +} diff --git a/internal/fileedit/editor_test.go b/internal/fileedit/editor_test.go new file mode 100644 index 0000000..c1f6226 --- /dev/null +++ b/internal/fileedit/editor_test.go @@ -0,0 +1,246 @@ +package fileedit + +import ( + "context" + "encoding/json" + "errors" + "testing" +) + +// fakeRunner stands in for the cluster: it records the JobParams the Editor +// rendered and replays a canned payload as if a Pod had printed it. +type fakeRunner struct { + calls int + got []JobParams + payload []byte + err error +} + +func (f *fakeRunner) Run(_ context.Context, p JobParams) ([]byte, error) { + f.calls++ + f.got = append(f.got, p) + return f.payload, f.err +} + +func mustPayload(t *testing.T, res Result) []byte { + t.Helper() + b, err := json.Marshal(res) + if err != nil { + t.Fatalf("marshal: %v", err) + } + return b +} + +// TestEditorRendersParams checks the Editor projects each operation onto the +// JobParams the renderer expects — in particular that the world PVC comes from the +// shared naming convention rather than being assembled locally, which is what keeps +// the editor pointed at the same volume the operator created and the reaper deletes. +func TestEditorRendersParams(t *testing.T) { + t.Run("list", func(t *testing.T) { + r := &fakeRunner{payload: mustPayload(t, Result{Entries: []Entry{{Name: "a"}}})} + e := &Editor{Runner: r, Config: Config{Image: "img"}} + + entries, truncated, err := e.List(context.Background(), "survival", "config") + if err != nil { + t.Fatalf("List: %v", err) + } + if len(entries) != 1 || truncated { + t.Fatalf("entries=%+v truncated=%v", entries, truncated) + } + p := r.got[0] + if p.Op != OpList || p.Path != "config" || p.Server != "survival" { + t.Fatalf("params = %+v", p) + } + if p.WorldPVC != "world-survival-0" { + t.Fatalf("WorldPVC = %q, want the naming convention's world-survival-0", p.WorldPVC) + } + if p.WorldsRoot != "/data" { + t.Fatalf("WorldsRoot = %q, want the default /data", p.WorldsRoot) + } + if len(p.Content) != 0 { + t.Fatal("a list must carry no content") + } + }) + + t.Run("read", func(t *testing.T) { + r := &fakeRunner{payload: mustPayload(t, Result{Content: []byte("motd=hi\n")})} + e := &Editor{Runner: r, Config: Config{Image: "img"}} + + got, err := e.Read(context.Background(), "survival", "server.properties") + if err != nil { + t.Fatalf("Read: %v", err) + } + if string(got) != "motd=hi\n" { + t.Fatalf("content = %q", got) + } + if r.got[0].Op != OpRead || r.got[0].Path != "server.properties" { + t.Fatalf("params = %+v", r.got[0]) + } + }) + + t.Run("write", func(t *testing.T) { + r := &fakeRunner{payload: mustPayload(t, Result{})} + e := &Editor{Runner: r, Config: Config{Image: "img"}} + + if err := e.Write(context.Background(), "survival", "ops.json", []byte("[]")); err != nil { + t.Fatalf("Write: %v", err) + } + if r.got[0].Op != OpWrite || string(r.got[0].Content) != "[]" { + t.Fatalf("params = %+v", r.got[0]) + } + }) +} + +// TestEditorMintsAFreshOpID guards the RBAC-forced invariant from the other side: +// the Job name is unique per invocation only because the Editor mints a new id +// every time. If it ever cached one, two operations would collide on a name +// felis-api has no permission to delete. +func TestEditorMintsAFreshOpID(t *testing.T) { + r := &fakeRunner{payload: mustPayload(t, Result{})} + e := &Editor{Runner: r, Config: Config{Image: "img"}} + + for range 3 { + if _, err := e.Read(context.Background(), "survival", "x"); err != nil { + t.Fatalf("Read: %v", err) + } + } + seen := map[string]bool{} + for _, p := range r.got { + if p.OpID == "" { + t.Fatal("op id must never be empty") + } + if seen[p.OpID] { + t.Fatalf("op id %q reused across invocations", p.OpID) + } + seen[p.OpID] = true + } +} + +// TestEditorMapsResultCodes proves a caller-fault Result becomes the sentinel the +// API maps. The default branch matters most: an unrecognised code must FAIL rather +// than read as success, so a future Job version reporting a new failure mode cannot +// be silently mistaken for a completed operation. +func TestEditorMapsResultCodes(t *testing.T) { + cases := []struct { + name string + code string + want error + }{ + {"missing file", CodeNotFound, ErrNotFound}, + {"escaping path", CodeBadPath, ErrBadPath}, + {"oversized", CodeTooLarge, ErrTooLarge}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + r := &fakeRunner{payload: mustPayload(t, Result{Code: tc.code, Error: "detail here"})} + e := &Editor{Runner: r, Config: Config{Image: "img"}} + _, err := e.Read(context.Background(), "survival", "x") + if !errors.Is(err, tc.want) { + t.Fatalf("err = %v, want %v", err, tc.want) + } + }) + } + + t.Run("an unknown code still fails", func(t *testing.T) { + r := &fakeRunner{payload: mustPayload(t, Result{Code: "from_the_future", Error: "?"})} + e := &Editor{Runner: r, Config: Config{Image: "img"}} + if _, err := e.Read(context.Background(), "survival", "x"); err == nil { + t.Fatal("an unrecognised failure code must not read as success") + } + }) + + t.Run("a malformed payload is an error, not an empty success", func(t *testing.T) { + r := &fakeRunner{payload: []byte("not json at all")} + e := &Editor{Runner: r, Config: Config{Image: "img"}} + if _, err := e.Read(context.Background(), "survival", "x"); err == nil { + t.Fatal("a malformed result must fail") + } + }) + + t.Run("a runner failure propagates", func(t *testing.T) { + r := &fakeRunner{err: errors.New("pod never scheduled")} + e := &Editor{Runner: r, Config: Config{Image: "img"}} + if _, err := e.Read(context.Background(), "survival", "x"); err == nil { + t.Fatal("a runner error must propagate") + } + }) +} + +// TestEditorRefusesOversizedWriteBeforeTheCluster checks the size ceiling is applied +// before a Job is rendered. Letting it through would surface as an opaque etcd +// object-size rejection long after felis-api committed to the request. +func TestEditorRefusesOversizedWriteBeforeTheCluster(t *testing.T) { + r := &fakeRunner{payload: mustPayload(t, Result{})} + e := &Editor{Runner: r, Config: Config{Image: "img"}} + + err := e.Write(context.Background(), "survival", "big.txt", make([]byte, MaxWriteBytes+1)) + if !errors.Is(err, ErrTooLarge) { + t.Fatalf("err = %v, want ErrTooLarge", err) + } + if r.calls != 0 { + t.Fatal("an oversized write must never reach the cluster") + } +} + +// TestEditorNormalisesEmptyResults pins that "nothing there" is a success, not a +// nil surprise: an empty directory lists as [] and a zero-length file reads as +// empty bytes, so no caller has to distinguish nil from empty. +func TestEditorNormalisesEmptyResults(t *testing.T) { + r := &fakeRunner{payload: mustPayload(t, Result{})} + e := &Editor{Runner: r, Config: Config{Image: "img"}} + + entries, _, err := e.List(context.Background(), "survival", "empty") + if err != nil { + t.Fatalf("List: %v", err) + } + if entries == nil { + t.Fatal("an empty directory must list as [], not nil") + } + + content, err := e.Read(context.Background(), "survival", "empty.txt") + if err != nil { + t.Fatalf("Read: %v", err) + } + if content == nil { + t.Fatal("a zero-length file must read as empty bytes, not nil") + } +} + +// TestExtractResult covers the log-scanning half of the transport: pods/log merges +// stdout and stderr, so the payload must be found by its marker among arbitrary +// noise rather than by assuming the log is pure JSON. +func TestExtractResult(t *testing.T) { + t.Run("finds the payload among stderr noise", func(t *testing.T) { + log := "warning: something from the runtime\n" + + ResultPrefix + `{"content":"aGk="}` + "\n" + + "a trailing stderr line\n" + payload, ok := extractResult(log) + if !ok { + t.Fatal("payload not found") + } + var res Result + if err := json.Unmarshal(payload, &res); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if string(res.Content) != "hi" { + t.Fatalf("content = %q", res.Content) + } + }) + + t.Run("takes the last marked line", func(t *testing.T) { + log := ResultPrefix + `{"code":"bad_path"}` + "\n" + ResultPrefix + `{"content":"aGk="}` + "\n" + payload, ok := extractResult(log) + if !ok { + t.Fatal("payload not found") + } + if string(payload) != `{"content":"aGk="}` { + t.Fatalf("payload = %s, want the last marked line", payload) + } + }) + + t.Run("reports absence rather than guessing", func(t *testing.T) { + if _, ok := extractResult("no marker here\njust noise\n"); ok { + t.Fatal("a log with no marked line must report not-found") + } + }) +} diff --git a/internal/fileedit/exec.go b/internal/fileedit/exec.go new file mode 100644 index 0000000..9a63c4b --- /dev/null +++ b/internal/fileedit/exec.go @@ -0,0 +1,337 @@ +package fileedit + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "io/fs" + "os" + "path" + "time" +) + +// The three operations the editor supports. The set is deliberately closed and +// tiny: list a directory, read a file, write a file. There is no rename, delete, +// or chmod — each would need its own containment and audit story, and none is +// required to fix a broken server.properties, which is what this subsystem exists +// for. +// +// A write DOES accept arbitrary bytes at any path inside the mount, and that is a +// real capability rather than an oversight: the root is the server's whole working +// directory (see Config.WorldsRoot), so an owner can write plugins/.jar and +// Paper will load it on the next boot. It is the same power a hosting panel's file +// manager gives, scoped to a server the caller already owns and already controls +// through /command. Note what it is NOT scoped by: admin image curation. Images +// are admin-only (POST /images, POST /images/build) and modpack submissions need +// an admin verdict, so this is the one owner-tier route that lands executable code +// in a backend pod. That trade was made deliberately; if it is ever revisited, the +// guard belongs in write() below, which is the single choke point all three +// callers route through. +const ( + OpList = "list" + OpRead = "read" + OpWrite = "write" +) + +// Result codes. A failure that is the CALLER's fault travels back as a Result +// with a Code rather than as a non-zero exit, so felis-api can map it onto a +// precise 4xx (handlers_files.go) instead of collapsing every failure into "the +// Job died" 500. Only an infrastructure failure — the world mount unreadable, the +// result unprintable — exits non-zero. +const ( + CodeBadPath = "bad_path" // escapes the world root, is absolute, or is otherwise unopenable + CodeNotFound = "not_found" // resolves inside the root but nothing is there + CodeTooLarge = "too_large" // the file exceeds MaxReadBytes +) + +// ResultPrefix marks the single stdout line carrying the JSON Result. The Job's +// output is read back through the pods/log subresource, which returns the +// container's stdout and stderr MERGED — so a Go runtime warning, a libc message, +// or anything else the container writes to stderr lands in the same stream. The +// marker is what makes the payload findable in that mixed stream: felis-api scans +// for the last line carrying this prefix rather than assuming the log is pure +// JSON. Without it any stray stderr byte would corrupt every response. +const ResultPrefix = "FELIS-FILES-RESULT: " + +// ContentEnv is the environment variable the write path carries new file content +// in (base64). It travels on the Job spec felis-api creates, because felis-api +// holds `jobs: create` but NOT `secrets: create` in the minecraft namespace +// (internal/platform.APIMinecraftRole) — a Secret is not available to it, so the +// Job spec is the only channel into the Pod. The consequence is that written +// content is readable by anyone holding jobs:get in the minecraft namespace, +// which is a cluster-admin-level power; it is NOT readable by felis-operator, +// felis-reaper, or any weak Job SA, none of which hold that verb. +const ContentEnv = "FELIS_FILE_CONTENT" + +// Size and count ceilings. Every one of them exists because the result travels +// through a Kubernetes object or a pod log, neither of which is an unbounded pipe: +// +// - MaxWriteBytes bounds the env var on the Job spec. etcd refuses an object +// over ~1.5MiB, and the base64 of the content is ~4/3 of it, so 256KiB leaves +// an order of magnitude of headroom for the rest of the spec. Any real +// server.properties / ops.json / bukkit.yml is a few KiB. +// - MaxReadBytes bounds what a read pulls back through the pod log INTO +// felis-api's memory. Without it a caller could name a 500MiB region file and +// make the API buffer it — a trivial memory DoS from an ordinary owner-tier +// request. 1MiB comfortably covers every config file and refuses world data. +// - MaxEntries bounds a listing. A world's region/ directory legitimately holds +// thousands of .mca files, so this truncates rather than errors (Truncated +// says so), keeping the log line bounded while still being useful. +const ( + MaxWriteBytes = 256 << 10 // 256 KiB + MaxReadBytes = 1 << 20 // 1 MiB + MaxEntries = 2000 +) + +// Entry is one directory entry in a listing. It carries only what a file browser +// needs to render a row and decide whether the entry is descendable; mode bits, +// ownership, and inode data are deliberately absent — they are not actionable +// through this editor (there is no chmod/chown op) and would only widen what a +// listing discloses about the node. +type Entry struct { + Name string `json:"name"` + Size int64 `json:"size"` + IsDir bool `json:"is_dir"` + ModTime time.Time `json:"mod_time"` +} + +// Result is the single JSON object the Job prints and felis-api parses back. One +// shape covers all three ops so the transport has exactly one thing to find and +// unmarshal; the op decides which fields are populated. +// +// Content is []byte, so encoding/json base64-encodes it on the way out and +// decodes it on the way back with no hand-rolled codec. That is what makes the +// read path binary-safe: a config file with a stray non-UTF-8 byte round-trips +// intact instead of being mangled into U+FFFD by a string round-trip. +type Result struct { + // Code and Error are set together on a caller-fault failure; both empty means + // the op succeeded. + Code string `json:"code,omitempty"` + Error string `json:"error,omitempty"` + + Entries []Entry `json:"entries,omitempty"` + Content []byte `json:"content,omitempty"` + // Truncated reports that the listing hit MaxEntries and is incomplete, so a + // client renders "showing first N" rather than silently implying the directory + // is smaller than it is. + Truncated bool `json:"truncated,omitempty"` +} + +// Execute performs op on the file named by path, resolved inside root, and returns +// the Result to print. root is the in-Pod mount path of the server's world PVC; +// path is the caller-supplied relative path underneath it. +// +// CONTAINMENT INVARIANT: every filesystem access goes through *os.Root, never +// through a path string this function assembled. os.Root is the stdlib's +// escape-proof directory handle — it resolves each component against the open root +// descriptor and refuses any traversal that would leave it, whether by "..", by an +// absolute path, or by a SYMLINK pointing outside. That last case is why the +// string-prefix check in internal/backup/tarlocal.go is not reused here: a prefix +// test validates the path as text, then opens it as a path, and between those two +// steps a symlink can be swapped in (TOCTOU). A world directory holds +// attacker-influenced content — players create files through ordinary gameplay, +// and plugins create more — so a symlink escaping to /etc or to another server's +// mount is a live threat, not a theoretical one. os.Root closes it structurally: +// there is no window between the check and the open because they are the same +// operation. +// +// The path is passed to os.Root verbatim apart from mapping "" to ".". In +// particular an ABSOLUTE path is NOT rewritten into a relative one — it is handed +// to os.Root as-is and refused. Silently reinterpreting "/etc/passwd" as +// "/etc/passwd" would turn an unambiguous escape attempt into a successful +// read of a file the caller did not name, which is exactly the confusion this +// editor must not have. +func Execute(root, op, path string, content []byte) (Result, error) { + r, err := os.OpenRoot(root) + if err != nil { + // The world mount itself is unopenable: infrastructure, not caller fault. + return Result{}, fmt.Errorf("open world root %q: %w", root, err) + } + defer r.Close() + + if path == "" { + path = "." + } + + switch op { + case OpList: + return list(r, path), nil + case OpRead: + return read(r, path), nil + case OpWrite: + return write(r, path, content), nil + default: + return Result{}, fmt.Errorf("unknown op %q", op) + } +} + +// list reads one directory. It does not recurse: a browser asks for one level at +// a time, and recursion would make both the result size and the traversal cost +// unbounded in a world directory. +func list(r *os.Root, path string) Result { + f, err := r.Open(path) + if err != nil { + return failure(err, path) + } + defer f.Close() + + // ReadDir(MaxEntries+1) reads one MORE than the ceiling so the overflow is + // detectable without walking the whole directory: if the extra entry came back, + // the listing is truncated. io.EOF means the directory ended within the limit. + dirents, err := f.ReadDir(MaxEntries + 1) + if err != nil && !errors.Is(err, io.EOF) { + return failure(err, path) + } + + truncated := len(dirents) > MaxEntries + if truncated { + dirents = dirents[:MaxEntries] + } + + entries := make([]Entry, 0, len(dirents)) + for _, de := range dirents { + e := Entry{Name: de.Name(), IsDir: de.IsDir()} + // Info() can fail on an entry deleted between the ReadDir and the stat (a + // running plugin rotating a log, say). That is not a reason to fail the whole + // listing, so the entry is reported with a zero size/mtime rather than dropped + // — a name that exists is still useful to the caller. + if info, err := de.Info(); err == nil { + e.Size, e.ModTime = info.Size(), info.ModTime() + } + entries = append(entries, e) + } + return Result{Entries: entries, Truncated: truncated} +} + +// secretConfigPath is the one file in a world mount holding PLATFORM secret +// material rather than the owner's own configuration. felis-lobby's entrypoint +// writes FELIS_FORWARDING_SECRET into it on every boot, and that value is +// identical on every backend in the cluster — operator.buildEnv injects one Secret +// everywhere. Reading it out of a server you own would therefore hand you the +// Velocity modern-forwarding handshake key for EVERYONE's servers: cross-tenant +// material that merely happens to sit in your volume. Every other path here is the +// caller's own data, which is why this is the only denial. +// +// Refusing it costs no legitimate repair. The entrypoint rewrites the file whole +// on every boot and its own header says "Do not hand-edit", so an edit made +// through this editor could never survive a restart anyway. Only the READ is +// denied; a write is left alone because writing the file leaks nothing and is +// equally futile. +// +// ponytail: an exact match on one cleaned path, not a pattern. This is the whole +// known exposure — grep FELIS_FORWARDING_SECRET across deploy/ — and if another +// image ever persists a platform secret into the mount, add its path here rather +// than inventing a matcher. +const secretConfigPath = "config/paper-global.yml" + +// read returns a file's bytes. It stats first so an oversized file is refused +// BEFORE any of it is buffered — checking after the read would mean the memory +// blow-up this ceiling exists to prevent has already happened. +func read(r *os.Root, name string) Result { + // path.Clean, not a raw compare: "./config/paper-global.yml", + // "config//paper-global.yml" and "config/../config/paper-global.yml" all name + // the same file, and a string equality test would wave every one of them + // through. Cleaning collapses them to the single canonical form this matches. + // Slash-based path (not filepath) is correct because the Job container is always + // Linux, whatever the developer machine rendering the spec runs. + if path.Clean(name) == secretConfigPath { + return Result{Code: CodeBadPath, Error: fmt.Sprintf( + "%s holds the proxy forwarding secret, which is shared cluster-wide, and is not readable through the editor", name)} + } + + f, err := r.Open(name) + if err != nil { + return failure(err, name) + } + defer f.Close() + + info, err := f.Stat() + if err != nil { + return failure(err, name) + } + if info.IsDir() { + return Result{Code: CodeBadPath, Error: fmt.Sprintf("%s is a directory, not a file", name)} + } + if info.Size() > MaxReadBytes { + return Result{Code: CodeTooLarge, Error: fmt.Sprintf( + "%s is %d bytes; the editor reads at most %d", name, info.Size(), MaxReadBytes)} + } + + // LimitReader is belt-and-braces against the file growing between the Stat and + // the read: the ceiling then holds on the bytes actually buffered, not merely on + // the size observed a moment earlier. + b, err := io.ReadAll(io.LimitReader(f, MaxReadBytes)) + if err != nil { + return failure(err, name) + } + return Result{Content: b} +} + +// write replaces a file's contents. It truncates rather than appends, and it does +// NOT create parent directories: every path this editor writes is an existing +// config file being corrected, so an unexpected mkdir would more likely be a typo +// materialising a stray directory in the world mount than an intent. +// +// O_CREATE is still allowed so a config file the server has not yet generated can +// be authored. os.Root applies the same containment to the create as to an open, +// so a symlink at the target pointing outside the root is refused rather than +// followed — the classic "write through a planted symlink" escape. +func write(r *os.Root, path string, content []byte) Result { + if len(content) > MaxWriteBytes { + // Defence in depth: felis-api already refuses an oversized write with a 413 + // before rendering the Job. Re-checking here keeps the ceiling true even if + // this entrypoint is ever driven directly. + return Result{Code: CodeTooLarge, Error: fmt.Sprintf( + "content is %d bytes; the editor writes at most %d", len(content), MaxWriteBytes)} + } + f, err := r.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0o644) + if err != nil { + return failure(err, path) + } + if _, err := f.Write(content); err != nil { + f.Close() + return failure(err, path) + } + // Close is where a buffered-write error surfaces, so its error is honoured + // rather than deferred-and-dropped: reporting success on a write that did not + // land would leave the caller believing a broken config was fixed. + if err := f.Close(); err != nil { + return failure(err, path) + } + return Result{} +} + +// failure maps a filesystem error onto a caller-facing Result code. Anything that +// is genuinely "nothing is there" becomes not_found; EVERYTHING else — including +// every os.Root containment refusal — becomes bad_path. +// +// That default is deliberate. os.Root reports an escape as a *fs.PathError with no +// exported sentinel to match on, so the mapping cannot test for "escaped" +// positively; it tests for the one benign case it can name and refuses the rest. +// Failing closed this way means a future os.Root error kind is reported as a bad +// path rather than leaking through as a success. +// +// The error text is included because it is generated by the stdlib from the +// caller's OWN path inside their OWN world mount, so it discloses nothing they +// could not learn by listing — and it is the difference between a usable "no such +// file" and an opaque 400. +func failure(err error, path string) Result { + if errors.Is(err, fs.ErrNotExist) { + return Result{Code: CodeNotFound, Error: fmt.Sprintf("%s does not exist", path)} + } + return Result{Code: CodeBadPath, Error: err.Error()} +} + +// Print writes r as the single marked stdout line the Job's reader looks for. The +// JSON is written with no indentation on purpose: the payload must occupy exactly +// ONE log line, because the reader identifies it by a line prefix. An indented +// encoding would split it across lines and make it unfindable. +func Print(w io.Writer, res Result) error { + b, err := json.Marshal(res) + if err != nil { + return fmt.Errorf("marshal result: %w", err) + } + _, err = fmt.Fprintf(w, "%s%s\n", ResultPrefix, b) + return err +} diff --git a/internal/fileedit/exec_test.go b/internal/fileedit/exec_test.go new file mode 100644 index 0000000..c570f20 --- /dev/null +++ b/internal/fileedit/exec_test.go @@ -0,0 +1,357 @@ +package fileedit + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// worldRoot builds a throwaway world directory with a couple of files and returns +// its path, plus the path of a sibling directory OUTSIDE it holding a secret. The +// sibling stands in for /etc — anything the editor must never reach — so an escape +// that succeeds is observable as the secret's contents coming back, not merely as +// a missing error. +func worldRoot(t *testing.T) (root, outside string) { + t.Helper() + base := t.TempDir() + root = filepath.Join(base, "world") + outside = filepath.Join(base, "outside") + for _, d := range []string{root, outside, filepath.Join(root, "config")} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatalf("mkdir %s: %v", d, err) + } + } + write := func(p, content string) { + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatalf("write %s: %v", p, err) + } + } + write(filepath.Join(root, "server.properties"), "motd=hello\n") + write(filepath.Join(root, "config", "paper.yml"), "verbose: false\n") + write(filepath.Join(outside, "secret.txt"), "TOP-SECRET") + return root, outside +} + +// TestExecuteContainment is the security test of this package. The world directory +// holds attacker-influenced content (players and plugins create files in it), so +// each vector below is a path a caller could genuinely supply to try to leave the +// world mount. Every one MUST be refused — a refusal is CodeBadPath (or, where the +// kernel resolves it to nothing at all, CodeNotFound), never a successful read. +// +// The assertion is deliberately doubled: the Result must carry a failure Code AND +// the secret's contents must not appear in it. Checking only the code would pass a +// hypothetical future regression that returned a code alongside populated content. +func TestExecuteContainment(t *testing.T) { + root, outside := worldRoot(t) + + // A symlink INSIDE the world pointing OUTSIDE it — the vector a string-prefix + // check cannot stop and the reason this package uses os.Root. The link is a + // perfectly ordinary file to a prefix test ("world/escape-link" is under + // "world/"), yet opening it lands on the secret. + if err := os.Symlink(outside, filepath.Join(root, "escape-link")); err != nil { + t.Skipf("symlinks unavailable on this platform: %v", err) + } + // A symlink pointing at an absolute path outside the root, planted at the exact + // name a caller would then "read" — the write-through-a-planted-symlink shape. + if err := os.Symlink(filepath.Join(outside, "secret.txt"), filepath.Join(root, "planted.txt")); err != nil { + t.Fatalf("symlink: %v", err) + } + + vectors := []struct { + name string + path string + }{ + {"parent traversal", "../outside/secret.txt"}, + {"nested parent traversal", "config/../../outside/secret.txt"}, + {"absolute path", filepath.Join(outside, "secret.txt")}, + {"absolute path to etc", "/etc/passwd"}, + {"symlinked directory", "escape-link/secret.txt"}, + {"symlinked file", "planted.txt"}, + {"traversal past the filesystem root", "../../../../../../etc/passwd"}, + } + + for _, v := range vectors { + t.Run("read "+v.name, func(t *testing.T) { + res, err := Execute(root, OpRead, v.path, nil) + if err != nil { + t.Fatalf("Execute returned an infrastructure error, want a contained refusal: %v", err) + } + if res.Code == "" { + t.Fatalf("path %q was ALLOWED (content=%q) — containment breached", v.path, res.Content) + } + if bytes.Contains(res.Content, []byte("TOP-SECRET")) { + t.Fatalf("path %q leaked out-of-root content despite code %q", v.path, res.Code) + } + }) + } + + // The write side must be contained by the same invariant: a planted symlink + // must not become a write into the file it points at. + t.Run("write through a planted symlink is refused", func(t *testing.T) { + res, err := Execute(root, OpWrite, "planted.txt", []byte("pwned")) + if err != nil { + t.Fatalf("Execute: %v", err) + } + if res.Code == "" { + t.Fatal("write through a symlink leaving the root was ALLOWED") + } + b, err := os.ReadFile(filepath.Join(outside, "secret.txt")) + if err != nil { + t.Fatalf("read secret: %v", err) + } + if string(b) != "TOP-SECRET" { + t.Fatalf("out-of-root file was MODIFIED through the symlink: %q", b) + } + }) + + t.Run("write escaping by traversal is refused", func(t *testing.T) { + res, err := Execute(root, OpWrite, "../outside/new.txt", []byte("pwned")) + if err != nil { + t.Fatalf("Execute: %v", err) + } + if res.Code == "" { + t.Fatal("write via ../ was ALLOWED") + } + if _, err := os.Stat(filepath.Join(outside, "new.txt")); err == nil { + t.Fatal("a file was created outside the world root") + } + }) + + t.Run("list escaping by traversal is refused", func(t *testing.T) { + res, err := Execute(root, OpList, "../outside", nil) + if err != nil { + t.Fatalf("Execute: %v", err) + } + if res.Code == "" { + t.Fatalf("listing outside the root was ALLOWED: %+v", res.Entries) + } + }) +} + +// TestExecuteHappyPath proves the three ops actually work inside the root, so the +// containment test above cannot be trivially satisfied by a function that refuses +// everything. +func TestExecuteHappyPath(t *testing.T) { + root, _ := worldRoot(t) + + t.Run("list the world root", func(t *testing.T) { + res, err := Execute(root, OpList, "", nil) + if err != nil { + t.Fatalf("Execute: %v", err) + } + if res.Code != "" { + t.Fatalf("unexpected failure %s: %s", res.Code, res.Error) + } + got := map[string]Entry{} + for _, e := range res.Entries { + got[e.Name] = e + } + if _, ok := got["server.properties"]; !ok { + t.Fatalf("server.properties missing from listing: %+v", res.Entries) + } + if e, ok := got["config"]; !ok || !e.IsDir { + t.Fatalf("config should be listed as a directory: %+v", got["config"]) + } + if e := got["server.properties"]; e.Size != int64(len("motd=hello\n")) { + t.Fatalf("size = %d, want %d", e.Size, len("motd=hello\n")) + } + }) + + t.Run("list a subdirectory", func(t *testing.T) { + res, err := Execute(root, OpList, "config", nil) + if err != nil { + t.Fatalf("Execute: %v", err) + } + if res.Code != "" || len(res.Entries) != 1 || res.Entries[0].Name != "paper.yml" { + t.Fatalf("unexpected listing: %+v (code %q)", res.Entries, res.Code) + } + }) + + t.Run("read a file", func(t *testing.T) { + res, err := Execute(root, OpRead, "server.properties", nil) + if err != nil { + t.Fatalf("Execute: %v", err) + } + if res.Code != "" || string(res.Content) != "motd=hello\n" { + t.Fatalf("content = %q, code = %q", res.Content, res.Code) + } + }) + + t.Run("write replaces content, then reads back", func(t *testing.T) { + if res, err := Execute(root, OpWrite, "server.properties", []byte("motd=changed\n")); err != nil || res.Code != "" { + t.Fatalf("write failed: %v / %+v", err, res) + } + b, err := os.ReadFile(filepath.Join(root, "server.properties")) + if err != nil { + t.Fatalf("read back: %v", err) + } + if string(b) != "motd=changed\n" { + t.Fatalf("on-disk content = %q, want the written bytes", b) + } + }) + + t.Run("write creates a new file but not parent directories", func(t *testing.T) { + if res, err := Execute(root, OpWrite, "ops.json", []byte("[]")); err != nil || res.Code != "" { + t.Fatalf("creating a new file should succeed: %v / %+v", err, res) + } + res, err := Execute(root, OpWrite, "nope/deep.txt", []byte("x")) + if err != nil { + t.Fatalf("Execute: %v", err) + } + if res.Code == "" { + t.Fatal("writing into a non-existent directory should fail, not mkdir it") + } + }) + + t.Run("missing file reads as not_found", func(t *testing.T) { + res, err := Execute(root, OpRead, "absent.txt", nil) + if err != nil { + t.Fatalf("Execute: %v", err) + } + if res.Code != CodeNotFound { + t.Fatalf("code = %q, want %q", res.Code, CodeNotFound) + } + }) + + t.Run("reading a directory is bad_path, not a garbled read", func(t *testing.T) { + res, err := Execute(root, OpRead, "config", nil) + if err != nil { + t.Fatalf("Execute: %v", err) + } + if res.Code != CodeBadPath { + t.Fatalf("code = %q, want %q", res.Code, CodeBadPath) + } + }) + + t.Run("oversized write is refused", func(t *testing.T) { + res, err := Execute(root, OpWrite, "big.txt", make([]byte, MaxWriteBytes+1)) + if err != nil { + t.Fatalf("Execute: %v", err) + } + if res.Code != CodeTooLarge { + t.Fatalf("code = %q, want %q", res.Code, CodeTooLarge) + } + }) + + t.Run("oversized read is refused before buffering", func(t *testing.T) { + if err := os.WriteFile(filepath.Join(root, "huge.bin"), make([]byte, MaxReadBytes+1), 0o644); err != nil { + t.Fatalf("write huge: %v", err) + } + res, err := Execute(root, OpRead, "huge.bin", nil) + if err != nil { + t.Fatalf("Execute: %v", err) + } + if res.Code != CodeTooLarge { + t.Fatalf("code = %q, want %q", res.Code, CodeTooLarge) + } + }) +} + +// TestReadIsBinarySafe proves the []byte/base64 round-trip preserves bytes that a +// string round-trip would destroy. A config file with a stray non-UTF-8 byte must +// come back byte-identical, or "read, edit one line, write" would silently corrupt +// the rest of the file. +func TestReadIsBinarySafe(t *testing.T) { + root, _ := worldRoot(t) + raw := []byte{0xff, 0xfe, 'o', 'k', 0x00, 0x80} + if err := os.WriteFile(filepath.Join(root, "raw.bin"), raw, 0o644); err != nil { + t.Fatalf("write raw: %v", err) + } + + res, err := Execute(root, OpRead, "raw.bin", nil) + if err != nil || res.Code != "" { + t.Fatalf("read failed: %v / %+v", err, res) + } + + // Round-trip through the wire encoding, which is how felis-api actually receives it. + var buf bytes.Buffer + if err := Print(&buf, res); err != nil { + t.Fatalf("Print: %v", err) + } + line := strings.TrimPrefix(strings.TrimSpace(buf.String()), ResultPrefix) + var back Result + if err := json.Unmarshal([]byte(line), &back); err != nil { + t.Fatalf("unmarshal: %v", err) + } + if !bytes.Equal(back.Content, raw) { + t.Fatalf("content = % x, want % x", back.Content, raw) + } +} + +// TestPrintIsOneMarkedLine pins the transport contract: felis-api finds the payload +// by scanning merged stdout+stderr for ResultPrefix, so the payload must be exactly +// one line and must carry the marker. An indented encoder would break the reader. +func TestPrintIsOneMarkedLine(t *testing.T) { + var buf bytes.Buffer + if err := Print(&buf, Result{Entries: []Entry{{Name: "a"}, {Name: "b"}}}); err != nil { + t.Fatalf("Print: %v", err) + } + out := buf.String() + if !strings.HasPrefix(out, ResultPrefix) { + t.Fatalf("output lacks the marker: %q", out) + } + if n := strings.Count(strings.TrimSuffix(out, "\n"), "\n"); n != 0 { + t.Fatalf("payload spans %d extra lines; it must be exactly one", n) + } +} + +// TestReadRefusesTheForwardingSecret pins the one path denial in this package. The +// value in config/paper-global.yml is the SAME on every backend in the cluster, so +// a read here is not a caller reading their own data — it is the Velocity handshake +// key for everyone else's servers. +// +// The equivalent-spelling cases are the substance of this test. A bare string +// compare against the constant would pass the first case and wave through all the +// rest, which is exactly the bug this guards; each alternative below names the same +// file to the kernel, so each must be refused identically. +func TestReadRefusesTheForwardingSecret(t *testing.T) { + root, _ := worldRoot(t) + const secret = "secret: aVeryRealForwardingKey" + if err := os.WriteFile(filepath.Join(root, "config", "paper-global.yml"), + []byte(secret), 0o644); err != nil { + t.Fatalf("seed paper-global.yml: %v", err) + } + + for _, spelling := range []string{ + "config/paper-global.yml", + "./config/paper-global.yml", + "config//paper-global.yml", + "config/../config/paper-global.yml", + "config/./paper-global.yml", + } { + res, err := Execute(root, OpRead, spelling, nil) + if err != nil { + t.Fatalf("%s: Execute: %v", spelling, err) + } + if res.Code != CodeBadPath { + t.Errorf("%s: code = %q, want %q — an equivalent spelling must not bypass the denial", + spelling, res.Code, CodeBadPath) + } + if strings.Contains(string(res.Content), "aVeryRealForwardingKey") { + t.Errorf("%s: the forwarding secret leaked into the result", spelling) + } + } + + // The denial is READ-only and exact: a neighbouring file in the same directory + // stays readable, or the guard would have broken ordinary config repair. + res, err := Execute(root, OpRead, "config/paper.yml", nil) + if err != nil { + t.Fatalf("Execute: %v", err) + } + if res.Code != "" { + t.Errorf("config/paper.yml: code = %q, want success — the denial must not widen", res.Code) + } + + // Writing it is still allowed: it leaks nothing, and the lobby entrypoint + // rewrites the file whole on every boot regardless. + res, err = Execute(root, OpWrite, "config/paper-global.yml", []byte("proxies: {}\n")) + if err != nil { + t.Fatalf("Execute: %v", err) + } + if res.Code != "" { + t.Errorf("write code = %q, want success — only the read is denied", res.Code) + } +} diff --git a/internal/fileedit/jobspec.go b/internal/fileedit/jobspec.go new file mode 100644 index 0000000..2fb7102 --- /dev/null +++ b/internal/fileedit/jobspec.go @@ -0,0 +1,249 @@ +package fileedit + +import ( + "encoding/base64" + "fmt" + "time" + + batchv1 "k8s.io/api/batch/v1" + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +// Label keys applied to file-editor objects, mirroring internal/restore and +// internal/backupjob so all three world-touching executors are observable the same +// way. LabelOpID is the one addition: it is how felis-api finds THIS invocation's +// Pod among any others, which matters here in a way it does not for restore — +// file operations are interactive and repeated, so several may be in flight or +// lingering inside their TTL at once. +const ( + LabelManagedBy = "app.kubernetes.io/managed-by" + LabelComponent = "app.kubernetes.io/component" + LabelServer = "felis.lolicon.best/server" + LabelOpID = "felis.lolicon.best/files-op" + + managedByValue = "felis-files" + componentValue = "world-files" + + worldVolume = "world" + felisBinaryPath = "/usr/local/bin/felis" + containerName = "files" +) + +// JobParams are the rendered inputs to a file-editor Job, derived from a server + +// operation + Config by the Editor. jobspec is a pure function of them so the +// security-critical Job shape is unit-tested without a cluster. +type JobParams struct { + Server string + // OpID is the per-invocation identifier that both names the Job and labels its + // Pod. See Editor.run for why every invocation gets a fresh one. + OpID string + Op string + Path string + Content []byte // OpWrite only + WorldPVC string + + Namespace string + ServiceAccount string + Image string + WorldsRoot string + Deadline time.Duration + CPULimit string + MemLimit string + RunAsUser int64 + RunAsGroup int64 + FSGroup int64 + + TTLAfterFinished time.Duration +} + +// FilesJobName is the Job name for one file operation. Unlike RestoreJobName it is +// NOT a pure function of the server: it carries the per-invocation OpID. +// +// That difference is forced by RBAC and is the single most load-bearing decision in +// this package. felis-api holds `jobs: create` in the minecraft namespace and +// NOTHING else — no jobs:get, no jobs:delete (internal/platform.APIMinecraftRole). +// So it can neither poll a Job nor clean one up; ttlSecondsAfterFinished is the only +// reclamation. With a deterministic name the FIRST file operation would leave a +// completed Job squatting the name for the whole TTL window, and every subsequent +// operation would collide with it — and, unable to delete it, the editor would be +// wedged until the TTL expired. A restore can accept that (it runs once per +// incident); an editor cannot (browse a directory, open a file, save it — three +// operations in as many seconds). internal/backupjob reached the same conclusion for +// the same reason. +func FilesJobName(server, opID string) string { return "files-" + server + "-" + opID } + +func filesLabels(p JobParams) map[string]string { + return map[string]string{ + LabelManagedBy: managedByValue, + LabelComponent: componentValue, + LabelServer: p.Server, + LabelOpID: p.OpID, + } +} + +// FilesJob renders the file-editor Job. Its isolation is the strictest of the three +// world executors — a strict SUBSET of what a restore Pod gets — and every guarantee +// is asserted by jobspec_test.go, because no cluster runs in this environment: +// +// - runs under the weak felis-restore SA (never the felis-api SA) with its token +// auto-mount disabled, so it cannot reach the K8s API (spec §16, §21). It reuses +// that bare, Role-less SA for the same reason internal/backupjob does: this Pod +// needs no K8s API access at all, so a second identity with the same (empty) +// powers would be a manifest to maintain for no isolation gain; +// - mounts EXACTLY ONE volume — the world PVC — and NO Secret, NO ConfigMap, and +// NO backup PVC. It is therefore strictly blinder than the backup Pod, which +// mounts the config Secret to self-record its row: a file-editor Pod has nothing +// to record, so it is handed no database URL and no credential of any kind (the +// four-power red line, spec §22); +// - mounts that one volume READ-ONLY for list and read. Only a write needs to +// mutate the world, so two of the three operations physically cannot — the +// kernel refuses, not merely the code. This is why readOnly is derived from the +// op rather than fixed; +// - runs as a non-root, fixed uid/gid with an fsGroup matching the operator's +// StatefulSet, so a file this Pod writes is owned by the identity the minecraft +// server later runs as — a config file the server cannot read would be worse +// than no edit at all; +// - no privilege, no privilege escalation, read-only root filesystem, drop ALL +// capabilities. The world mount is the only writable path, and only on a write; +// - activeDeadlineSeconds + backoffLimit=0 so a wedged mount cannot loop or hang +// forever; ttlSecondsAfterFinished GCs the finished Job, which — see +// FilesJobName — is the ONLY cleanup available to felis-api. +// +// The container runs `/usr/local/bin/felis files` (cmd/felis), which performs the +// operation under os.Root containment and prints the marked JSON Result line that +// felis-api reads back through pods/log. +func FilesJob(p JobParams) (*batchv1.Job, error) { + if p.Image == "" { + return nil, fmt.Errorf("fileedit: image is empty") + } + if p.WorldPVC == "" { + return nil, fmt.Errorf("fileedit: world PVC name is required") + } + if p.OpID == "" { + return nil, fmt.Errorf("fileedit: op id is required") + } + if p.Op != OpList && p.Op != OpRead && p.Op != OpWrite { + return nil, fmt.Errorf("fileedit: unknown op %q", p.Op) + } + if len(p.Content) > MaxWriteBytes { + return nil, fmt.Errorf("fileedit: content is %d bytes, over the %d limit", len(p.Content), MaxWriteBytes) + } + limits, err := resourceLimits(p.CPULimit, p.MemLimit) + if err != nil { + return nil, err + } + deadline := int64(p.Deadline / time.Second) + if deadline <= 0 { + deadline = int64(defaultDeadline / time.Second) + } + ttl := int32(p.TTLAfterFinished / time.Second) + if ttl <= 0 { + ttl = int32(defaultTTL / time.Second) + } + + // Only a write may mutate the world. Mounting read-only for the other two ops + // makes "a listing cannot damage a world" a kernel guarantee rather than a + // code-review one. + readOnlyWorld := p.Op != OpWrite + + container := corev1.Container{ + Name: containerName, + Image: p.Image, + Command: []string{felisBinaryPath, "files"}, + Args: []string{ + "--op", p.Op, + "--path", p.Path, + "--worlds-root", p.WorldsRoot, + }, + VolumeMounts: []corev1.VolumeMount{ + {Name: worldVolume, MountPath: p.WorldsRoot, ReadOnly: readOnlyWorld}, + }, + Resources: corev1.ResourceRequirements{Limits: limits, Requests: limits}, + SecurityContext: &corev1.SecurityContext{ + Privileged: boolPtr(false), + AllowPrivilegeEscalation: boolPtr(false), + ReadOnlyRootFilesystem: boolPtr(true), + Capabilities: &corev1.Capabilities{Drop: []corev1.Capability{"ALL"}}, + }, + } + + // New content rides the Job spec as a base64 env var. felis-api cannot create a + // Secret (it holds secrets:get only), so the spec is the sole channel into the + // Pod; base64 keeps arbitrary bytes — CRLF line endings, a UTF-8 BOM, a binary + // blob — intact through a field that must be a valid string. The env var is set + // ONLY for a write, so a list/read Job spec carries no caller content at all. + if p.Op == OpWrite { + container.Env = []corev1.EnvVar{{ + Name: ContentEnv, + Value: base64.StdEncoding.EncodeToString(p.Content), + }} + } + + job := &batchv1.Job{ + ObjectMeta: metav1.ObjectMeta{ + Name: FilesJobName(p.Server, p.OpID), + Namespace: p.Namespace, + Labels: filesLabels(p), + }, + Spec: batchv1.JobSpec{ + // One shot: a file operation that failed must surface its failure, not be + // retried behind the caller's back — a retried write is a second write. + BackoffLimit: int32Ptr(0), + ActiveDeadlineSeconds: int64Ptr(deadline), + TTLSecondsAfterFinished: int32Ptr(ttl), + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{Labels: filesLabels(p)}, + Spec: corev1.PodSpec{ + RestartPolicy: corev1.RestartPolicyNever, + ServiceAccountName: p.ServiceAccount, + AutomountServiceAccountToken: boolPtr(false), + SecurityContext: &corev1.PodSecurityContext{ + RunAsNonRoot: boolPtr(true), + RunAsUser: int64Ptr(p.RunAsUser), + RunAsGroup: int64Ptr(p.RunAsGroup), + FSGroup: int64Ptr(p.FSGroup), + }, + Containers: []corev1.Container{container}, + Volumes: []corev1.Volume{{ + Name: worldVolume, + VolumeSource: corev1.VolumeSource{ + PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ + ClaimName: p.WorldPVC, + ReadOnly: readOnlyWorld, + }, + }, + }}, + }, + }, + }, + } + return job, nil +} + +// resourceLimits parses the CPU/memory limits into a ResourceList. +func resourceLimits(cpu, mem string) (corev1.ResourceList, error) { + if cpu == "" { + cpu = defaultCPULimit + } + if mem == "" { + mem = defaultMemLimit + } + cpuQty, err := resource.ParseQuantity(cpu) + if err != nil { + return nil, fmt.Errorf("fileedit: invalid cpu limit %q: %w", cpu, err) + } + memQty, err := resource.ParseQuantity(mem) + if err != nil { + return nil, fmt.Errorf("fileedit: invalid memory limit %q: %w", mem, err) + } + return corev1.ResourceList{ + corev1.ResourceCPU: cpuQty, + corev1.ResourceMemory: memQty, + }, nil +} + +func boolPtr(b bool) *bool { return &b } +func int32Ptr(i int32) *int32 { return &i } +func int64Ptr(i int64) *int64 { return &i } diff --git a/internal/fileedit/jobspec_test.go b/internal/fileedit/jobspec_test.go new file mode 100644 index 0000000..59943fc --- /dev/null +++ b/internal/fileedit/jobspec_test.go @@ -0,0 +1,259 @@ +package fileedit + +import ( + "encoding/base64" + "strings" + "testing" + "time" + + corev1 "k8s.io/api/core/v1" +) + +func testParams(op string) JobParams { + return JobParams{ + Server: "survival", + OpID: "deadbeefcafe0001", + Op: op, + Path: "server.properties", + WorldPVC: "world-survival-0", + Namespace: "minecraft", + ServiceAccount: "felis-restore", + Image: "registry.example/felis:v1", + WorldsRoot: "/data", + Deadline: 2 * time.Minute, + CPULimit: "500m", + MemLimit: "256Mi", + RunAsUser: 1000, + RunAsGroup: 1000, + FSGroup: 1000, + TTLAfterFinished: 2 * time.Minute, + } +} + +// TestFilesJobIsolation asserts every isolation guarantee FilesJob documents. No +// cluster runs in this environment, so this pure-function test IS the enforcement: +// if someone loosens the Pod spec, this is what catches it. +func TestFilesJobIsolation(t *testing.T) { + job, err := FilesJob(testParams(OpRead)) + if err != nil { + t.Fatalf("FilesJob: %v", err) + } + spec := job.Spec.Template.Spec + + t.Run("runs under the weak SA with its token un-mounted", func(t *testing.T) { + if spec.ServiceAccountName != "felis-restore" { + t.Fatalf("SA = %q, want the weak felis-restore", spec.ServiceAccountName) + } + if spec.AutomountServiceAccountToken == nil || *spec.AutomountServiceAccountToken { + t.Fatal("the SA token MUST NOT be auto-mounted — the Pod must not reach the K8s API") + } + }) + + // The four-power red line: a file-editor Pod holds no credential of any kind. It + // is strictly blinder than the backup Pod, which does mount the config Secret. + t.Run("mounts exactly one volume and no credential", func(t *testing.T) { + if len(spec.Volumes) != 1 { + t.Fatalf("volumes = %d, want exactly 1 (the world PVC)", len(spec.Volumes)) + } + v := spec.Volumes[0] + if v.PersistentVolumeClaim == nil || v.PersistentVolumeClaim.ClaimName != "world-survival-0" { + t.Fatalf("the sole volume must be the world PVC, got %+v", v) + } + if v.Secret != nil || v.ConfigMap != nil || v.Projected != nil { + t.Fatalf("no Secret/ConfigMap/Projected volume may be mounted, got %+v", v) + } + }) + + t.Run("runs non-root with the operator's runtime identity", func(t *testing.T) { + sc := spec.SecurityContext + if sc == nil || sc.RunAsNonRoot == nil || !*sc.RunAsNonRoot { + t.Fatal("RunAsNonRoot must be true") + } + // FSGroup must match the minecraft server's group or a file this Pod writes + // would be unreadable by the server that later mounts the same volume. + if sc.RunAsUser == nil || *sc.RunAsUser != 1000 || + sc.RunAsGroup == nil || *sc.RunAsGroup != 1000 || + sc.FSGroup == nil || *sc.FSGroup != 1000 { + t.Fatalf("uid/gid/fsGroup must all be 1000, got %+v", sc) + } + }) + + t.Run("container drops every privilege", func(t *testing.T) { + if len(spec.Containers) != 1 { + t.Fatalf("containers = %d, want 1", len(spec.Containers)) + } + sc := spec.Containers[0].SecurityContext + if sc == nil { + t.Fatal("the container needs a SecurityContext") + } + if sc.Privileged == nil || *sc.Privileged { + t.Fatal("Privileged must be false") + } + if sc.AllowPrivilegeEscalation == nil || *sc.AllowPrivilegeEscalation { + t.Fatal("AllowPrivilegeEscalation must be false") + } + if sc.ReadOnlyRootFilesystem == nil || !*sc.ReadOnlyRootFilesystem { + t.Fatal("ReadOnlyRootFilesystem must be true") + } + if sc.Capabilities == nil || len(sc.Capabilities.Drop) != 1 || sc.Capabilities.Drop[0] != "ALL" { + t.Fatalf("capabilities must drop ALL, got %+v", sc.Capabilities) + } + }) + + t.Run("is one-shot, deadlined, and self-collecting", func(t *testing.T) { + if job.Spec.BackoffLimit == nil || *job.Spec.BackoffLimit != 0 { + t.Fatal("BackoffLimit must be 0 — a retried write is a second write") + } + if job.Spec.ActiveDeadlineSeconds == nil || *job.Spec.ActiveDeadlineSeconds != 120 { + t.Fatalf("ActiveDeadlineSeconds = %v, want 120", job.Spec.ActiveDeadlineSeconds) + } + // The TTL is the ONLY cleanup available: felis-api holds no jobs:delete. + if job.Spec.TTLSecondsAfterFinished == nil || *job.Spec.TTLSecondsAfterFinished != 120 { + t.Fatalf("TTLSecondsAfterFinished = %v, want 120", job.Spec.TTLSecondsAfterFinished) + } + if spec.RestartPolicy != corev1.RestartPolicyNever { + t.Fatalf("RestartPolicy = %q, want Never", spec.RestartPolicy) + } + }) + + t.Run("runs the files entrypoint with the op as arguments", func(t *testing.T) { + c := spec.Containers[0] + if len(c.Command) != 2 || c.Command[0] != felisBinaryPath || c.Command[1] != "files" { + t.Fatalf("command = %v, want [%s files]", c.Command, felisBinaryPath) + } + args := strings.Join(c.Args, " ") + for _, want := range []string{"--op read", "--path server.properties", "--worlds-root /data"} { + if !strings.Contains(args, want) { + t.Fatalf("args %q missing %q", args, want) + } + } + }) +} + +// TestFilesJobWorldMountIsReadOnlyExceptForWrite pins the guarantee that only a +// write can mutate a world. For list and read the kernel refuses the write, not +// merely the code — a defence that survives a bug in the entrypoint. +func TestFilesJobWorldMountIsReadOnlyExceptForWrite(t *testing.T) { + cases := []struct { + op string + wantReadOnly bool + }{ + {OpList, true}, + {OpRead, true}, + {OpWrite, false}, + } + for _, tc := range cases { + t.Run(tc.op, func(t *testing.T) { + job, err := FilesJob(testParams(tc.op)) + if err != nil { + t.Fatalf("FilesJob: %v", err) + } + spec := job.Spec.Template.Spec + gotMount := spec.Containers[0].VolumeMounts[0].ReadOnly + gotVol := spec.Volumes[0].PersistentVolumeClaim.ReadOnly + if gotMount != tc.wantReadOnly || gotVol != tc.wantReadOnly { + t.Fatalf("op %s: mount.readOnly=%v volume.readOnly=%v, want %v", + tc.op, gotMount, gotVol, tc.wantReadOnly) + } + }) + } +} + +// TestFilesJobContentEnv pins the write channel: content rides the Job spec +// base64-encoded, and ONLY for a write — a list or read Job spec must carry no +// caller content at all. +func TestFilesJobContentEnv(t *testing.T) { + t.Run("write carries base64 content", func(t *testing.T) { + p := testParams(OpWrite) + p.Content = []byte("motd=hello\n\x00\xff") + job, err := FilesJob(p) + if err != nil { + t.Fatalf("FilesJob: %v", err) + } + env := job.Spec.Template.Spec.Containers[0].Env + if len(env) != 1 || env[0].Name != ContentEnv { + t.Fatalf("env = %+v, want exactly %s", env, ContentEnv) + } + got, err := base64.StdEncoding.DecodeString(env[0].Value) + if err != nil { + t.Fatalf("env value is not base64: %v", err) + } + if string(got) != string(p.Content) { + t.Fatalf("decoded %q, want %q — arbitrary bytes must survive", got, p.Content) + } + // The content must never leak into argv, which is world-readable on the node. + if strings.Contains(strings.Join(job.Spec.Template.Spec.Containers[0].Args, " "), "motd=hello") { + t.Fatal("content must not appear in the container arguments") + } + }) + + for _, op := range []string{OpList, OpRead} { + t.Run(op+" carries no content env", func(t *testing.T) { + job, err := FilesJob(testParams(op)) + if err != nil { + t.Fatalf("FilesJob: %v", err) + } + if env := job.Spec.Template.Spec.Containers[0].Env; len(env) != 0 { + t.Fatalf("env = %+v, want none for a %s", env, op) + } + }) + } +} + +// TestFilesJobNameIsPerInvocation is the RBAC-forced property documented on +// FilesJobName. felis-api holds jobs:create and NOTHING else — no jobs:delete — so +// a deterministic name would let the first completed Job squat it for a whole TTL +// window and wedge every subsequent operation. Two operations on the same server +// must therefore never collide. +func TestFilesJobNameIsPerInvocation(t *testing.T) { + a := testParams(OpRead) + b := testParams(OpRead) + b.OpID = "deadbeefcafe0002" + + ja, err := FilesJob(a) + if err != nil { + t.Fatalf("FilesJob: %v", err) + } + jb, err := FilesJob(b) + if err != nil { + t.Fatalf("FilesJob: %v", err) + } + if ja.Name == jb.Name { + t.Fatalf("two operations on one server share the Job name %q — the editor would wedge", ja.Name) + } + if !strings.Contains(ja.Name, "survival") || !strings.Contains(ja.Name, a.OpID) { + t.Fatalf("job name %q should carry the server and the op id", ja.Name) + } + // The op id must also label the Pod, or the runner could not select THIS + // operation's Pod to read its result from. + if got := ja.Spec.Template.ObjectMeta.Labels[LabelOpID]; got != a.OpID { + t.Fatalf("pod label %s = %q, want %q", LabelOpID, got, a.OpID) + } +} + +// TestFilesJobRejectsBadParams checks the renderer fails loudly rather than +// producing a Job that cannot run or that would be refused by etcd. +func TestFilesJobRejectsBadParams(t *testing.T) { + cases := []struct { + name string + mutate func(*JobParams) + }{ + {"no image", func(p *JobParams) { p.Image = "" }}, + {"no world PVC", func(p *JobParams) { p.WorldPVC = "" }}, + {"no op id", func(p *JobParams) { p.OpID = "" }}, + {"unknown op", func(p *JobParams) { p.Op = "delete" }}, + {"oversized content", func(p *JobParams) { + p.Op, p.Content = OpWrite, make([]byte, MaxWriteBytes+1) + }}, + {"bad cpu limit", func(p *JobParams) { p.CPULimit = "half" }}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + p := testParams(OpRead) + tc.mutate(&p) + if _, err := FilesJob(p); err == nil { + t.Fatal("expected an error") + } + }) + } +} diff --git a/internal/fileedit/k8sjobs.go b/internal/fileedit/k8sjobs.go new file mode 100644 index 0000000..8f68039 --- /dev/null +++ b/internal/fileedit/k8sjobs.go @@ -0,0 +1,187 @@ +package fileedit + +import ( + "bufio" + "context" + "errors" + "fmt" + "io" + "strings" + "time" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" +) + +// pollInterval is how often the runner re-Lists Pods while waiting for the file +// Job to finish. It is a POLL rather than a Watch because felis-api holds +// pods:list and NOT pods:watch (internal/platform.APIMinecraftRole) — establishing +// a watch would need a permission this design exists to avoid. Half a second is +// well inside the human-perceptible floor for an operation already dominated by +// Pod scheduling, while keeping the request count on a slow image pull modest. +const pollInterval = 500 * time.Millisecond + +// maxLogBytes bounds what the runner will buffer from a Pod's log. The payload is +// at most a base64-encoded MaxReadBytes (≈4/3 of 1 MiB) plus the JSON envelope, so +// 4 MiB is generous headroom while still refusing to let a Pod that floods stderr +// pull felis-api's memory down with it. +const maxLogBytes = 4 << 20 + +// K8sRunner is the production Runner (spec §7, §16). It drives one file operation +// end to end using ONLY the three permissions felis-api already holds in the +// minecraft namespace, which is the entire point of the design: +// +// jobs:create → create the file Job +// pods:list → find its Pod and observe the phase (no pods:get, no pods:watch) +// pods/log:get → read the printed result back +// +// It deliberately takes a typed kubernetes.Interface rather than the +// controller-runtime client that internal/restore's K8sJobs uses: the log +// subresource (GetLogs(...).Stream) exists only on the typed CoreV1 client, and +// the Job create is available on both — so one client covers all three calls +// instead of the binding carrying two. +// +// INTEGRATION-ONLY: like K8sLogStreamer and K8sCluster this needs a live cluster; +// it compiles here but is exercised only against one, never by the hermetic test +// suite. The Oracle verifies the layer above it (Editor orchestration and error +// mapping) against a fake Runner, and the Job shape via the pure jobspec. +type K8sRunner struct { + cs kubernetes.Interface +} + +// NewK8sRunner builds a Runner over cs. Every per-operation parameter — the +// namespace included — travels in the JobParams the Editor renders, so there is no +// Config to retain here. +func NewK8sRunner(cs kubernetes.Interface) *K8sRunner { + return &K8sRunner{cs: cs} +} + +// Run creates the file Job, waits for its Pod to reach a terminal phase, and +// returns the JSON payload from the ResultPrefix line of that Pod's log. +// +// The Job name carries a fresh random OpID (FilesJobName), so a create collision is +// not an expected condition the way it is for restore — an AlreadyExists here means +// a 64-bit collision inside one TTL window and is reported rather than absorbed, +// because absorbing it would mean returning ANOTHER operation's output. +func (k *K8sRunner) Run(ctx context.Context, p JobParams) ([]byte, error) { + job, err := FilesJob(p) + if err != nil { + return nil, err + } + if _, err := k.cs.BatchV1().Jobs(p.Namespace).Create(ctx, job, metav1.CreateOptions{}); err != nil { + return nil, fmt.Errorf("fileedit: create file job: %w", err) + } + + pod, err := k.awaitPod(ctx, p) + if err != nil { + return nil, err + } + + log, err := k.podLog(ctx, p.Namespace, pod.Name) + if err != nil { + return nil, err + } + + payload, ok := extractResult(log) + if !ok { + // No marked line: the entrypoint died before printing (an unmountable volume, + // an OOM kill, a deadline). The log tail travels in the error for the operator's + // benefit — this error reaches felis-api's logs, while the caller gets the + // generic 500 writeError produces, so no node detail leaks to the browser. + return nil, fmt.Errorf("fileedit: file job %s produced no result (phase %s): %s", + job.Name, pod.Status.Phase, tail(log)) + } + return payload, nil +} + +// awaitPod polls until the operation's Pod reaches a terminal phase. It selects by +// the per-invocation LabelOpID, so it can never observe a different operation's Pod +// — the reason that label exists. +// +// Both Succeeded and Failed are terminal and BOTH return the Pod rather than an +// error, because a caller-fault result (a path that escapes the root, a file that +// is too large) is printed and then exited on cleanly, and even a genuinely failed +// Pod may have printed a diagnosable result first. Deciding what the outcome MEANS +// is the caller's job (Run reads the printed result); this function only decides +// when there is nothing left to wait for. +func (k *K8sRunner) awaitPod(ctx context.Context, p JobParams) (*corev1.Pod, error) { + ticker := time.NewTicker(pollInterval) + defer ticker.Stop() + + for { + pods, err := k.cs.CoreV1().Pods(p.Namespace).List(ctx, metav1.ListOptions{ + LabelSelector: LabelOpID + "=" + p.OpID, + }) + if err != nil { + return nil, fmt.Errorf("fileedit: find file job pod: %w", err) + } + for i := range pods.Items { + switch pods.Items[i].Status.Phase { + case corev1.PodSucceeded, corev1.PodFailed: + return &pods.Items[i], nil + } + } + + select { + case <-ticker.C: + case <-ctx.Done(): + // The Editor's Timeout (or the client disconnecting) fired. The Job is left + // alone deliberately: felis-api holds no jobs:delete, and the Job's own + // activeDeadlineSeconds plus ttlSecondsAfterFinished retire it without help. + return nil, fmt.Errorf("fileedit: timed out waiting for the file job to finish: %w", ctx.Err()) + } + } +} + +// podLog reads a finished Pod's log. Follow is off — the Pod has already +// terminated, so the log is complete and a follow would merely block until the +// stream closed. +func (k *K8sRunner) podLog(ctx context.Context, namespace, pod string) (string, error) { + stream, err := k.cs.CoreV1().Pods(namespace).GetLogs(pod, &corev1.PodLogOptions{ + Container: containerName, + }).Stream(ctx) + if err != nil { + return "", fmt.Errorf("fileedit: read file job log: %w", err) + } + defer stream.Close() + + b, err := io.ReadAll(io.LimitReader(stream, maxLogBytes)) + if err != nil && !errors.Is(err, io.EOF) { + return "", fmt.Errorf("fileedit: read file job log: %w", err) + } + return string(b), nil +} + +// extractResult finds the marked payload in a Pod log. It scans for the LAST line +// carrying ResultPrefix because pods/log returns stdout and stderr MERGED: a Go +// runtime warning or a libc message can appear anywhere in the stream, so the +// payload must be located by its marker rather than by position. Taking the last +// match rather than the first is the conservative choice — if a marker somehow +// appeared more than once, the final one is the operation's actual outcome. +func extractResult(log string) ([]byte, bool) { + var payload string + var found bool + sc := bufio.NewScanner(strings.NewReader(log)) + sc.Buffer(make([]byte, 0, 64*1024), maxLogBytes) + for sc.Scan() { + if rest, ok := strings.CutPrefix(sc.Text(), ResultPrefix); ok { + payload, found = rest, true + } + } + if !found { + return nil, false + } + return []byte(payload), true +} + +// tail returns the last few hundred bytes of a log for an error message, so a +// diagnostic is useful without embedding an entire log in an error string. +func tail(log string) string { + const n = 512 + log = strings.TrimSpace(log) + if len(log) <= n { + return log + } + return "..." + log[len(log)-n:] +}