Unverified Commit 07973a4b authored by Lemon-miaow's avatar Lemon-miaow
Browse files

fix: restrict platform maintenance and backup visibility to Owner

parent 285c5695
Loading
Loading
Loading
Loading
+8 −8
Changes for docs/openapi.yaml: 8 added lines, 8 removed lines.
Original line number Diff line number Diff line
@@ -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':
+8 −8
Changes for internal/api/api.go: 8 added lines, 8 removed lines.
Original line number Diff line number Diff line
@@ -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
+13 −22
Changes for internal/api/handlers_updates_test.go: 13 added lines, 22 removed lines.
Original line number Diff line number Diff line
@@ -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)
			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 code = %d, want %d (%s)", get.Code, tc.want, get.Body.String())
					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)
+8 −8
Changes for panel/dev/mockApi.ts: 8 added lines, 8 removed lines.
Original line number Diff line number Diff line
@@ -1073,8 +1073,8 @@ async function handleSession(ctx: SessionContext): Promise<boolean> {
      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<boolean> {
      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<boolean> {
      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);
+16 −0
Changes for panel/e2e/startup-platform.smoke.spec.ts: 16 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -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();
});
Loading