From 07973a4bbd03457787fb5b523540ca844e544b9f Mon Sep 17 00:00:00 2001 From: Lemon-miaow Date: Wed, 7 Oct 2026 16:47:57 +0800 Subject: [PATCH] fix: restrict platform maintenance and backup visibility to Owner --- docs/openapi.yaml | 16 +++++----- internal/api/api.go | 16 +++++----- internal/api/handlers_updates_test.go | 37 +++++++++--------------- panel/dev/mockApi.ts | 16 +++++----- panel/e2e/startup-platform.smoke.spec.ts | 16 ++++++++++ panel/src/App.tsx | 2 +- panel/src/lib/nav.test.ts | 7 +++++ panel/src/lib/nav.ts | 5 ++-- panel/src/lib/openapi.gen.ts | 8 ++--- 9 files changed, 68 insertions(+), 55 deletions(-) diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 5f4a844..5e1e04b 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -4603,7 +4603,7 @@ paths: get: tags: [admin-updates] operationId: getUpdateWindow - summary: Read the SysAdmin-set auto-update maintenance window (admin). + summary: Read the Owner-set auto-update maintenance window (Owner). description: >- Felis applies no update on its own. `felis update` checks versions and prints an explicit apply command. `felis update --apply` reads this @@ -4612,7 +4612,7 @@ paths: An unreadable window is always a refusal, including with `--now` or `--force`. An unset window reads back as {start:null,end:null}. x-felis-face: [external] - x-felis-tier: admin + x-felis-tier: owner security: [{ sessionCookie: [] }] responses: '200': @@ -4628,7 +4628,7 @@ paths: put: tags: [admin-updates] operationId: setUpdateWindow - summary: Set or clear the SysAdmin auto-update maintenance window (admin). + summary: Set or clear the SysAdmin auto-update maintenance window (Owner). description: >- Persist the maintenance window as an absolute [start,end) interval. Both ends must be set with end strictly after start, or both null to clear the @@ -4638,7 +4638,7 @@ paths: setting a window only permits an apply inside it; outside, a Scheduled component degrades to notify. x-felis-face: [external] - x-felis-tier: admin + x-felis-tier: owner security: [{ sessionCookie: [] }] requestBody: required: true @@ -4664,14 +4664,14 @@ paths: get: tags: [admin-updates] operationId: getDBBackup - summary: Freshness of the newest control-plane database backup (admin). + summary: Freshness of the newest control-plane database backup (Owner). description: >- What the host's felis-db-backup.timer (or a manual `felis db backup`) last recorded in platform_settings. last is null before the first backup; stale is true then, and whenever the newest daily backup (last.daily_at) is missing or older than max_age_seconds. Read-only: backups run on the host, never through the API. x-felis-face: [external] - x-felis-tier: admin + x-felis-tier: owner security: [{ sessionCookie: [] }] responses: '200': @@ -4689,7 +4689,7 @@ paths: get: tags: [admin-updates] operationId: getUpdateReport - summary: The newest recorded version check of every tracked component (admin). + summary: The newest recorded version check of every tracked component (Owner). description: >- What `felis update --record` last stored in platform_settings; the installer's felis-update-check.timer runs it daily on the host, where the @@ -4697,7 +4697,7 @@ paths: stale is true then, and whenever the check is older than max_age_seconds. Read-only: Felis applies no update on its own. x-felis-face: [external] - x-felis-tier: admin + x-felis-tier: owner security: [{ sessionCookie: [] }] responses: '200': diff --git a/internal/api/api.go b/internal/api/api.go index 6b8d062..d8cc130 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -783,16 +783,16 @@ func (a *API) externalAPIRoutes() []apiRoute { // lives inside it, so approval would otherwise be blind. {Method: "GET", Pattern: "/api/v1/submissions/{id}/context", Admin: true, h: a.handleAdminSubmissionContext}, // Auto-update maintenance window (spec §B; decision core internal/updates). - // Admin-tier: it governs whether Felis may apply an update to itself, so setting - // it requires the admin Zero-Trust path, not a mere session. Advisory: `felis - // update` on the host reads it and warns before an apply outside it. - {Method: "GET", Pattern: "/api/v1/updates/window", Admin: true, h: a.handleGetUpdateWindow}, - {Method: "PUT", Pattern: "/api/v1/updates/window", Admin: true, h: a.handleSetUpdateWindow}, + // Owner-tier: it governs whether Felis may apply an update to itself, so setting + // it requires Owner access through the operator host. `felis update --apply` + // checks the configured window before backup and installation. + {Method: "GET", Pattern: "/api/v1/updates/window", Admin: true, Owner: true, h: a.handleGetUpdateWindow}, + {Method: "PUT", Pattern: "/api/v1/updates/window", Admin: true, Owner: true, h: a.handleSetUpdateWindow}, // The newest version check felis-update-check.timer recorded on the host. - {Method: "GET", Pattern: "/api/v1/updates/report", Admin: true, h: a.handleGetUpdateReport}, + {Method: "GET", Pattern: "/api/v1/updates/report", Admin: true, Owner: true, h: a.handleGetUpdateReport}, // Control-plane database backup freshness, as the host's felis-db-backup.timer - // last recorded it. Admin-tier: it names the host backup directory. - {Method: "GET", Pattern: "/api/v1/platform/db-backup", Admin: true, h: a.handleGetDBBackup}, + // last recorded it. Owner-tier: it names the host backup directory. + {Method: "GET", Pattern: "/api/v1/platform/db-backup", Admin: true, Owner: true, h: a.handleGetDBBackup}, // User admin (spec §7, owner-only). Every route gates on the admin Zero-Trust // path AND the owner role: listing, mutating, disabling, or deleting users is diff --git a/internal/api/handlers_updates_test.go b/internal/api/handlers_updates_test.go index 5893e3d..9c20976 100644 --- a/internal/api/handlers_updates_test.go +++ b/internal/api/handlers_updates_test.go @@ -6,24 +6,14 @@ import ( "testing" ) -// Auto-update maintenance-window admin API tests (task #38; decision core -// internal/updates). The load-bearing cases: an unset window and an explicitly -// cleared window both read back as {null,null} (two paths, one shape); a valid -// window round-trips through persistence; a half-set or inverted window is -// rejected fail-closed and never stored; and — the tier boundary — the routes are -// admin-only, gating precisely on IsAdmin() = role==admin AND the admin-access -// path, so neither a role=user nor an admin off the operator host can touch them. - -// seedUpdatesAPI returns an API whose external face authenticates every request as -// a full admin (role=admin AND ViaAdminAccess), so the admin-tier routes are -// reachable and the tests exercise the handler logic. Gating tests override -// api.External to vary the principal. +// Platform maintenance handlers require Owner access through the operator host. +// Handler tests use an Owner; boundary tests vary the principal explicitly. func seedUpdatesAPI(t *testing.T) (*API, *fakeRepo) { t.Helper() repo := newFakeRepo() api := newTestAPI(repo, newFakeCluster()) api.External = staticExternal{p: &Principal{ - UserID: "admin1", Email: "admin@" + testRoot, Role: "admin", ViaAdminAccess: true, + UserID: "owner1", Email: "owner@" + testRoot, Role: "owner", ViaAdminAccess: true, }} return api, repo } @@ -152,17 +142,16 @@ func TestUpdateWindowContentTypeGuard(t *testing.T) { } } -// TestUpdateWindowAdminGating proves the tier boundary discriminates on IsAdmin() -// precisely — role==admin AND the admin-access path — not on accident. A full admin -// reads 200; an admin who did NOT arrive via admin access, and a role=user player, -// are both 403 on both the GET and the mutating PUT. -func TestUpdateWindowAdminGating(t *testing.T) { +// Verify the Owner and operator-host boundaries for reads and writes. +func TestUpdateWindowOwnerGating(t *testing.T) { for _, tc := range []struct { name string p *Principal want int }{ - {"admin via admin-access", &Principal{UserID: "a1", Role: "admin", ViaAdminAccess: true}, http.StatusOK}, + {"owner via operator host", &Principal{UserID: "o1", Role: "owner", ViaAdminAccess: true}, http.StatusOK}, + {"owner off operator host", &Principal{UserID: "o1", Role: "owner", ViaAdminAccess: false}, http.StatusForbidden}, + {"admin via admin-access", &Principal{UserID: "a1", Role: "admin", ViaAdminAccess: true}, http.StatusForbidden}, {"admin off operator host", &Principal{UserID: "a1", Role: "admin", ViaAdminAccess: false}, http.StatusForbidden}, {"role=user player", &Principal{UserID: "u1", Role: "user", ViaAdminAccess: false}, http.StatusForbidden}, } { @@ -171,11 +160,13 @@ func TestUpdateWindowAdminGating(t *testing.T) { api := newTestAPI(repo, newFakeCluster()) api.External = staticExternal{p: tc.p} - get := do(api.ExternalHandler(), "GET", "/api/v1/updates/window", "", nil) - if get.Code != tc.want { - t.Fatalf("GET code = %d, want %d (%s)", get.Code, tc.want, get.Body.String()) + for _, path := range []string{"/api/v1/updates/window", "/api/v1/updates/report", "/api/v1/platform/db-backup"} { + get := do(api.ExternalHandler(), "GET", path, "", nil) + if get.Code != tc.want { + t.Fatalf("GET %s code = %d, want %d (%s)", path, get.Code, tc.want, get.Body.String()) + } } - // A valid PUT by a full admin is 200; a non-admin is 403 (adminOnly rejects + // A valid PUT by an Owner is 200; other identities are rejected // before the handler), so the wanted PUT code is the same as the GET's. put := do(api.ExternalHandler(), "PUT", "/api/v1/updates/window", `{"start":"2026-08-01T02:00:00Z","end":"2026-08-01T04:00:00Z"}`, jsonHeader) diff --git a/panel/dev/mockApi.ts b/panel/dev/mockApi.ts index 9aa13dd..c2792ab 100644 --- a/panel/dev/mockApi.ts +++ b/panel/dev/mockApi.ts @@ -1073,8 +1073,8 @@ async function handleSession(ctx: SessionContext): Promise { sendJSON(ctx.res, 200, { sources: [{ tag: "mojang", prefix: "", lookup_available: true }, ...ctx.state.authSources.sources.filter((source) => source.enabled).map((source) => ({ tag: source.tag, prefix: source.prefix, lookup_available: !!source.api_url || source.url.endsWith("/sessionserver/session/minecraft/hasJoined") }))] }); return true; case "GET platform/db-backup": { - if (!isAdmin(ctx.account.role)) { - sendError(ctx.res, 403, "forbidden", "admin account required"); + if (ctx.account.role !== "owner") { + sendError(ctx.res, 403, "forbidden", "owner account required"); return true; } // Yesterday's daily run: fresh, so the card shows its healthy state. @@ -1097,8 +1097,8 @@ async function handleSession(ctx: SessionContext): Promise { return true; } case "GET updates/report": { - if (!isAdmin(ctx.account.role)) { - sendError(ctx.res, 403, "forbidden", "admin account required"); + if (ctx.account.role !== "owner") { + sendError(ctx.res, 403, "forbidden", "owner account required"); return true; } // Last night's timer run: one update waiting, one feed unreachable, one @@ -1122,15 +1122,15 @@ async function handleSession(ctx: SessionContext): Promise { return true; } case "GET updates/window": - if (!isAdmin(ctx.account.role)) { - sendError(ctx.res, 403, "forbidden", "admin account required"); + if (ctx.account.role !== "owner") { + sendError(ctx.res, 403, "forbidden", "owner account required"); return true; } sendJSON(ctx.res, 200, ctx.state.updateWindow); return true; case "PUT updates/window": { - if (!isAdmin(ctx.account.role)) { - sendError(ctx.res, 403, "forbidden", "admin account required"); + if (ctx.account.role !== "owner") { + sendError(ctx.res, 403, "forbidden", "owner account required"); return true; } const body = await readJSON<{ start: string | null; end: string | null }>(ctx.req); diff --git a/panel/e2e/startup-platform.smoke.spec.ts b/panel/e2e/startup-platform.smoke.spec.ts index d30b341..f77721f 100644 --- a/panel/e2e/startup-platform.smoke.spec.ts +++ b/panel/e2e/startup-platform.smoke.spec.ts @@ -115,3 +115,19 @@ test("Owner executes node management and receives stage, failure logs and retry" await expect(page.getByText(t("admin:node_control_state_running"), { exact: true })).toBeVisible(); await expectFitsScreen(page); }); + +test("platform backup and version maintenance are Owner-only", async ({ page, signIn }) => { + await signIn("owner"); + expect((await page.request.patch("/api/v1/users/user", { data: { role: "admin" } })).ok()).toBe(true); + await signIn("user"); + await page.goto("/admin/updates"); + await expect(page.getByRole("navigation").getByRole("link", { name: t("navigation:admin_updates"), exact: true })).toHaveCount(0); + await expect(page.getByRole("heading", { name: t("admin:updates_title"), exact: true })).toHaveCount(0); + for (const path of ["updates/window", "updates/report", "platform/db-backup"]) { + expect((await page.request.get(`/api/v1/${path}`)).status()).toBe(403); + } + await signIn("owner"); + await page.goto("/admin/updates"); + await expect(page.getByRole("heading", { name: t("admin:updates_title"), exact: true })).toBeVisible(); + await expect(page.getByRole("navigation").getByRole("link", { name: t("navigation:admin_updates"), exact: true })).toBeVisible(); +}); diff --git a/panel/src/App.tsx b/panel/src/App.tsx index 4f82f7f..e5e931c 100644 --- a/panel/src/App.tsx +++ b/panel/src/App.tsx @@ -130,9 +130,9 @@ export default function App() { } /> } /> } /> - } /> {/* Owner-gated platform settings and user management. */} }> + } /> } /> } /> } /> diff --git a/panel/src/lib/nav.test.ts b/panel/src/lib/nav.test.ts index 9340f54..082113e 100644 --- a/panel/src/lib/nav.test.ts +++ b/panel/src/lib/nav.test.ts @@ -24,6 +24,13 @@ describe("visibleSections", () => { expect(ids).toEqual(["user", "admin"]); }); + it("places platform maintenance exclusively in Owner navigation, before platform settings", () => { + expect(visibleSections(true, false).flatMap((section) => section.items).some((item) => item.to === "/admin/updates")).toBe(false); + const owner = visibleSections(true, true).find((section) => section.id === "owner"); + expect(owner?.items.map((item) => item.to)).toContain("/admin/updates"); + expect(owner?.items.at(-1)?.to).toBe("/admin/platform"); + }); + it("keeps the User-Side section ungated so it survives both branches", () => { const user = NAV_SECTIONS.find((s) => s.id === "user"); expect(user?.adminOnly).toBe(false); diff --git a/panel/src/lib/nav.ts b/panel/src/lib/nav.ts index 84fb9f8..9f4079f 100644 --- a/panel/src/lib/nav.ts +++ b/panel/src/lib/nav.ts @@ -18,8 +18,7 @@ import { // identity see" decision is a pure function (visibleSections) that vitest can pin // without rendering React. The three sections map onto DESIGN-WEB-3SIDES §2: // User-Side is always present (app-tier); Admin-Side and SysAdmin-Side are a -// navigational separation of *concern* over the SAME admin tier — both gated by -// the one `is_admin` flag, surfaced as two sections only for admins. +// separation between server administration and Owner-only platform operations. // // Hiding a section is UX convenience, NOT a security control: every /admin and // /ops data call is independently 403-gated server-side (the RequireAdmin route @@ -68,7 +67,6 @@ export const NAV_SECTIONS: NavSection[] = [ { to: "/admin/images", key: "admin_images", icon: Boxes }, { to: "/admin/builds", key: "admin_builds", icon: Cpu }, { to: "/admin/submissions", key: "admin_submissions", icon: ClipboardCheck }, - { to: "/admin/updates", key: "admin_updates", icon: Clock }, ], }, { @@ -79,6 +77,7 @@ export const NAV_SECTIONS: NavSection[] = [ items: [ { to: "/admin/users", key: "admin_users", icon: Users }, { to: "/admin/auth-sources", key: "auth_sources", icon: ShieldCheck }, + { to: "/admin/updates", key: "admin_updates", icon: Clock }, { to: "/admin/platform", key: "platform_settings", icon: Settings }, ], }, diff --git a/panel/src/lib/openapi.gen.ts b/panel/src/lib/openapi.gen.ts index b59f8b6..7f535af 100644 --- a/panel/src/lib/openapi.gen.ts +++ b/panel/src/lib/openapi.gen.ts @@ -1404,12 +1404,12 @@ export interface paths { cookie?: never; }; /** - * Read the SysAdmin-set auto-update maintenance window (admin). + * Read the Owner-set auto-update maintenance window (Owner). * @description Felis applies no update on its own. `felis update` checks versions and prints an explicit apply command. `felis update --apply` reads this platform-wide [start,end) window before backup and before installation, refusing outside it unless `--now` explicitly starts manual maintenance. An unreadable window is always a refusal, including with `--now` or `--force`. An unset window reads back as {start:null,end:null}. */ get: operations["getUpdateWindow"]; /** - * Set or clear the SysAdmin auto-update maintenance window (admin). + * Set or clear the SysAdmin auto-update maintenance window (Owner). * @description Persist the maintenance window as an absolute [start,end) interval. Both ends must be set with end strictly after start, or both null to clear the window to unset. A half-set (exactly one end) or inverted/empty (end not after start) body is rejected 400, mirroring the decision core's fail-closed Window so a malformed schedule can never be stored. No forced auto-update: setting a window only permits an apply inside it; outside, a Scheduled component degrades to notify. */ put: operations["setUpdateWindow"]; @@ -1428,7 +1428,7 @@ export interface paths { cookie?: never; }; /** - * Freshness of the newest control-plane database backup (admin). + * Freshness of the newest control-plane database backup (Owner). * @description What the host's felis-db-backup.timer (or a manual `felis db backup`) last recorded in platform_settings. last is null before the first backup; stale is true then, and whenever the newest daily backup (last.daily_at) is missing or older than max_age_seconds. Read-only: backups run on the host, never through the API. */ get: operations["getDBBackup"]; @@ -1448,7 +1448,7 @@ export interface paths { cookie?: never; }; /** - * The newest recorded version check of every tracked component (admin). + * The newest recorded version check of every tracked component (Owner). * @description What `felis update --record` last stored in platform_settings; the installer's felis-update-check.timer runs it daily on the host, where the installed versions are readable. report is null before the first check; stale is true then, and whenever the check is older than max_age_seconds. Read-only: Felis applies no update on its own. */ get: operations["getUpdateReport"];