diff --git a/plugins/automations/locales/en.json b/plugins/automations/locales/en.json index b78a4ce12..b459b4ade 100644 --- a/plugins/automations/locales/en.json +++ b/plugins/automations/locales/en.json @@ -176,6 +176,7 @@ "title": "Maintenance", "description": "Pause host status, health, metric, container and tunnel-disconnection automations while work is in progress. Monitoring, manual runs and scheduled automations continue.", "denied": "You do not have permission to view maintenance.", + "sharedReadOnly": "This host was shared with you. Only its owner can change maintenance.", "failed": "Maintenance could not be updated.", "active": "Maintenance active", "estimated": "Estimated completion", diff --git a/plugins/automations/migrations/mysql/0004_rename_maintenance_index.sql b/plugins/automations/migrations/mysql/0004_rename_maintenance_index.sql new file mode 100644 index 000000000..156dd2ec9 --- /dev/null +++ b/plugins/automations/migrations/mysql/0004_rename_maintenance_index.sql @@ -0,0 +1,5 @@ +-- automations 0004: rename_maintenance_index +-- Renames the maintenance index to the idx_ pattern every other index uses. + +CREATE UNIQUE INDEX `idx_automation_maintenance_host_owner` ON `p_automations_host_maintenance` (`user_id`, `host_id`); +DROP INDEX `maintenance_host_owner` ON `p_automations_host_maintenance`; diff --git a/plugins/automations/migrations/postgres/0004_rename_maintenance_index.sql b/plugins/automations/migrations/postgres/0004_rename_maintenance_index.sql new file mode 100644 index 000000000..208c8c351 --- /dev/null +++ b/plugins/automations/migrations/postgres/0004_rename_maintenance_index.sql @@ -0,0 +1,5 @@ +-- automations 0004: rename_maintenance_index +-- Renames the maintenance index to the idx_ pattern every other index uses. + +CREATE UNIQUE INDEX IF NOT EXISTS "idx_automation_maintenance_host_owner" ON "p_automations_host_maintenance" ("user_id", "host_id"); +DROP INDEX IF EXISTS "maintenance_host_owner"; diff --git a/plugins/automations/migrations/snapshot.json b/plugins/automations/migrations/snapshot.json index 7ebde4115..22e6377ae 100644 --- a/plugins/automations/migrations/snapshot.json +++ b/plugins/automations/migrations/snapshot.json @@ -24,7 +24,7 @@ }, "indexes": [ { - "name": "maintenance_host_owner", + "name": "idx_automation_maintenance_host_owner", "columns": ["userId", "hostId"], "unique": true } diff --git a/plugins/automations/migrations/sqlite/0004_rename_maintenance_index.sql b/plugins/automations/migrations/sqlite/0004_rename_maintenance_index.sql new file mode 100644 index 000000000..208c8c351 --- /dev/null +++ b/plugins/automations/migrations/sqlite/0004_rename_maintenance_index.sql @@ -0,0 +1,5 @@ +-- automations 0004: rename_maintenance_index +-- Renames the maintenance index to the idx_ pattern every other index uses. + +CREATE UNIQUE INDEX IF NOT EXISTS "idx_automation_maintenance_host_owner" ON "p_automations_host_maintenance" ("user_id", "host_id"); +DROP INDEX IF EXISTS "maintenance_host_owner"; diff --git a/plugins/automations/src/backend/maintenance-routes.ts b/plugins/automations/src/backend/maintenance-routes.ts index bb03c133e..c75e79491 100644 --- a/plugins/automations/src/backend/maintenance-routes.ts +++ b/plugins/automations/src/backend/maintenance-routes.ts @@ -13,6 +13,7 @@ export function registerMaintenanceRoutes( * /plugin-api/automations/maintenance: * get: * summary: List maintenance windows on the caller's hosts + * description: Covers hosts the caller owns and hosts shared with them. A shared host carries its owner's maintenance with owned set to false. * tags: [Automations] * responses: * 200: { description: Maintenance state per host. } @@ -26,14 +27,25 @@ export function registerMaintenanceRoutes( const user = ctx.currentActor(); if (!user) return res.status(401).json({ error: "Authentication required" }); - const hosts = new Set( - (await ctx.hosts.list()) - .filter((host) => host.userId === user) - .map((host) => host.id), - ); - res.json( - (await service.list(user)).filter((row) => hosts.has(row.hostId)), - ); + // Maintenance belongs to the host's owner, so a shared host shows + // the owner's windows, read only. + const hostsByOwner = new Map>(); + for (const host of await ctx.hosts.list()) { + const ids = hostsByOwner.get(host.userId) ?? new Set(); + ids.add(host.id); + hostsByOwner.set(host.userId, ids); + } + const rows = []; + for (const [owner, ids] of hostsByOwner) { + for (const row of await service.list(owner)) { + if (!ids.has(row.hostId)) continue; + rows.push({ + hostId: row.hostId, + state: { ...row.state, owned: owner === user }, + }); + } + } + res.json(rows); } catch (error) { ctx.log.error("Failed to list maintenance", error as Error); res.status(500).json({ error: "Failed to list maintenance" }); @@ -49,9 +61,17 @@ export function registerMaintenanceRoutes( return res.status(401).json({ error: "Authentication required" }); try { const host = await ctx.hosts.get(hostId); - if (!host || host.userId !== user) - return res.status(404).json({ error: "Host not found" }); - if (!mutate) return res.json(await service.read(user, hostId)); + if (!host) return res.status(404).json({ error: "Host not found" }); + const owned = host.userId === user; + if (!mutate) + return res.json({ + ...(await service.read(host.userId, hostId)), + owned, + }); + if (!owned) + return res + .status(403) + .json({ error: "Only the host's owner can change maintenance" }); const state = await service.edit( user, hostId, @@ -65,7 +85,7 @@ export function registerMaintenanceRoutes( resourceName: host.name || host.ip, success: true, }); - res.json(state); + res.json({ ...state, owned: true }); } catch (error) { // Validation failures have no side effects; storage failures remain errors. if (error instanceof MaintenanceInputError) { @@ -87,8 +107,8 @@ export function registerMaintenanceRoutes( * required: true * schema: { type: integer } * responses: - * 200: { description: The active window, if any, and the planned ones. } - * 404: { description: Not one of the caller's hosts. } + * 200: { description: The active window, if any, and the planned ones. owned is false on a host shared with the caller. } + * 404: { description: A host the caller cannot see. } */ router.get( "/maintenance/:hostId", @@ -100,7 +120,7 @@ export function registerMaintenanceRoutes( * /plugin-api/automations/maintenance/{hostId}: * post: * summary: Start, schedule, end or remove maintenance on a host - * description: While a host is in maintenance its downtime does not raise alerts. Past the estimated end plus the grace period, it can alert again. + * description: While a host is in maintenance, automations with host status, health check, metric, Docker or tunnel triggers do not fire for it. Manual, scheduled, webhook and login triggers still run. Past the estimated end plus the grace period, an optional overdue alert is sent. * tags: [Automations] * parameters: * - in: path @@ -124,7 +144,8 @@ export function registerMaintenanceRoutes( * responses: * 200: { description: The new maintenance state. } * 400: { description: Invalid action or plan. } - * 404: { description: Not one of the caller's hosts. } + * 403: { description: The host was shared with the caller; only its owner can change maintenance. } + * 404: { description: A host the caller cannot see. } */ router.post( "/maintenance/:hostId", diff --git a/plugins/automations/src/backend/tables.ts b/plugins/automations/src/backend/tables.ts index 42786b2e8..9cd037d89 100644 --- a/plugins/automations/src/backend/tables.ts +++ b/plugins/automations/src/backend/tables.ts @@ -198,7 +198,10 @@ export const maintenanceTable = defineTable( }, { uniques: [ - { name: "maintenance_host_owner", columns: ["userId", "hostId"] }, + { + name: "idx_automation_maintenance_host_owner", + columns: ["userId", "hostId"], + }, ], }, ); diff --git a/plugins/automations/src/frontend/HostMaintenance.tsx b/plugins/automations/src/frontend/HostMaintenance.tsx index 49f0fdd89..6c57135c9 100644 --- a/plugins/automations/src/frontend/HostMaintenance.tsx +++ b/plugins/automations/src/frontend/HostMaintenance.tsx @@ -41,7 +41,7 @@ function VisibleBadge({ return ( {title} @@ -72,9 +72,11 @@ function MaintenanceEditor({ store: MaintenanceStore; }) { const { t } = useTranslation(); - const canEdit = usePermission("edit"); + const canEditPermission = usePermission("edit"); const states = useSyncExternalStore(store.subscribe, store.snapshot); const state = states[Number(host.id)]; + // A shared host shows its owner's maintenance, read only. + const canEdit = canEditPermission && state?.owned !== false; const [mode, setMode] = useState<"start" | "schedule">("start"); const [reason, setReason] = useState(""); const [start, setStart] = useState(""); @@ -174,6 +176,11 @@ function MaintenanceEditor({ ))} )} + {state?.owned === false && ( +

+ {t("maintenance.sharedReadOnly")} +

+ )} {canEdit && (
diff --git a/plugins/automations/src/frontend/maintenance-store.ts b/plugins/automations/src/frontend/maintenance-store.ts index ab621a9ed..7344d6df1 100644 --- a/plugins/automations/src/frontend/maintenance-store.ts +++ b/plugins/automations/src/frontend/maintenance-store.ts @@ -21,11 +21,16 @@ export function createMaintenanceStore(api: PluginApiClient) { "/maintenance", ); if (!disposed && version === revision) { - error = ""; - loaded = true; - snapshot = Object.fromEntries( + const next = Object.fromEntries( data.map(({ hostId, state }) => [hostId, state]), ); + // Most polls change nothing; skip the emit so badges do not re-render. + const unchanged = + loaded && !error && JSON.stringify(next) === JSON.stringify(snapshot); + error = ""; + loaded = true; + if (unchanged) return; + snapshot = next; emit(); } } catch { diff --git a/plugins/automations/src/maintenance.ts b/plugins/automations/src/maintenance.ts index bc2ddcae9..4d27ae056 100644 --- a/plugins/automations/src/maintenance.ts +++ b/plugins/automations/src/maintenance.ts @@ -23,6 +23,8 @@ export interface ActiveMaintenance { export interface HostMaintenance { active: ActiveMaintenance | null; plans: MaintenancePlan[]; + /** Set on API responses: false when the host was shared with the caller. */ + owned?: boolean; } export const emptyMaintenance = (): HostMaintenance => ({ diff --git a/plugins/automations/tests/backend/maintenance.test.ts b/plugins/automations/tests/backend/maintenance.test.ts index 90b6a39a3..3730aa1a2 100644 --- a/plugins/automations/tests/backend/maintenance.test.ts +++ b/plugins/automations/tests/backend/maintenance.test.ts @@ -28,14 +28,29 @@ const input = { }; describe("host maintenance", () => { - it("scopes reads and edits to the host owner and enforces edit permission", async () => { + it("shows a shared host the owner's maintenance read only and enforces edit permission", async () => { + // The mock serves every host to every user, as if alice shared host 1. server = await startServer({ hosts: [host(1), host(2, "bob")] }); - expect( - (await server.request("POST", "/maintenance/1", { body: input })).status, - ).toBe(200); - expect( - (await server.request("GET", "/maintenance", { user: "bob" })).body, - ).toEqual([]); + const started = await server.request("POST", "/maintenance/1", { + body: input, + }); + expect(started.status).toBe(200); + expect(started.body).toMatchObject({ owned: true }); + + const listed = ( + await server.request("GET", "/maintenance", { user: "bob" }) + ).body as Array<{ + hostId: number; + state: { owned: boolean; active: unknown }; + }>; + expect(listed).toHaveLength(1); + expect(listed[0].hostId).toBe(1); + expect(listed[0].state.owned).toBe(false); + expect(listed[0].state.active).toBeTruthy(); + + const read = await server.request("GET", "/maintenance/1", { user: "bob" }); + expect(read.status).toBe(200); + expect(read.body).toMatchObject({ owned: false }); expect( ( await server.request("POST", "/maintenance/1", { @@ -43,9 +58,9 @@ describe("host maintenance", () => { body: { action: "end" }, }) ).status, - ).toBe(404); + ).toBe(403); expect( - (await server.request("GET", "/maintenance/1", { user: "bob" })).status, + (await server.request("GET", "/maintenance/99", { user: "bob" })).status, ).toBe(404); expect( (await server.request("POST", "/maintenance/1x", { body: input })).status, diff --git a/plugins/automations/tests/backend/migrations.test.ts b/plugins/automations/tests/backend/migrations.test.ts index 48186f06f..5316957e9 100644 --- a/plugins/automations/tests/backend/migrations.test.ts +++ b/plugins/automations/tests/backend/migrations.test.ts @@ -224,4 +224,36 @@ describe("adopting the automation tables", () => { expect(count(db, "p_automations_automations")).toBe(0); expect(count(db, "p_automations_run_steps")).toBe(0); }); + + it("creates host maintenance with one row per owner and host, removed with either", async () => { + db = await createTestDb(pluginDir, { + before: (sqlite) => sqlite.exec(NOTIFICATION_CHANNELS_DDL), + }); + + const maintenanceIndexes = indexes(db, "p_automations_host_maintenance"); + expect(maintenanceIndexes).toContain( + "idx_automation_maintenance_host_owner", + ); + expect(maintenanceIndexes).not.toContain("maintenance_host_owner"); + + db.sqlite.exec(` + INSERT INTO users (id, username) VALUES ('alice', 'alice'); + INSERT INTO ssh_data (id) VALUES (1); + INSERT INTO ssh_data (id) VALUES (2); + INSERT INTO p_automations_host_maintenance (user_id, host_id, state) VALUES ('alice', 1, '{}'); + INSERT INTO p_automations_host_maintenance (user_id, host_id, state) VALUES ('alice', 2, '{}'); + `); + expect(() => + db.sqlite + .prepare( + "INSERT INTO p_automations_host_maintenance (user_id, host_id, state) VALUES ('alice', 1, '{}')", + ) + .run(), + ).toThrow(); + + db.sqlite.prepare("DELETE FROM ssh_data WHERE id = 1").run(); + expect(count(db, "p_automations_host_maintenance")).toBe(1); + db.sqlite.prepare("DELETE FROM users WHERE id = 'alice'").run(); + expect(count(db, "p_automations_host_maintenance")).toBe(0); + }); }); diff --git a/plugins/automations/tests/frontend/maintenance.test.tsx b/plugins/automations/tests/frontend/maintenance.test.tsx index d0a2f4ca5..1161bace3 100644 --- a/plugins/automations/tests/frontend/maintenance.test.tsx +++ b/plugins/automations/tests/frontend/maintenance.test.tsx @@ -71,6 +71,13 @@ describe("maintenance UI", () => { expect(view.queryByRole("button", { name: "End maintenance" })).toBeNull(); expect(view.queryByLabelText("Reason")).toBeNull(); }); + it("shows a shared host's maintenance read only even with edit permission", async () => { + const view = await render(apiFor({ ...active, owned: false })); + await waitFor(() => expect(view.getByText("OS upgrade")).toBeTruthy()); + expect(view.getByText(locales.maintenance.sharedReadOnly)).toBeTruthy(); + expect(view.queryByRole("button", { name: "End maintenance" })).toBeNull(); + expect(view.queryByLabelText("Reason")).toBeNull(); + }); it("submits a recurring UTC schedule with its grace period", async () => { const api = apiFor(emptyMaintenance()); const view = await render(api); diff --git a/src/backend/tests/fixtures/upgrade/upgrade-mysql.sql b/src/backend/tests/fixtures/upgrade/upgrade-mysql.sql index 8b48b835b..4ce2545e0 100644 --- a/src/backend/tests/fixtures/upgrade/upgrade-mysql.sql +++ b/src/backend/tests/fixtures/upgrade/upgrade-mysql.sql @@ -800,6 +800,13 @@ CREATE TABLE IF NOT EXISTS `p_automations_host_maintenance` ( ); CREATE UNIQUE INDEX `maintenance_host_owner` ON `p_automations_host_maintenance` (`user_id`, `host_id`); +-- ==== plugin automations 0004_rename_maintenance_index.sql ==== +-- automations 0004: rename_maintenance_index +-- Renames the maintenance index to the idx_ pattern every other index uses. + +CREATE UNIQUE INDEX `idx_automation_maintenance_host_owner` ON `p_automations_host_maintenance` (`user_id`, `host_id`); +DROP INDEX `maintenance_host_owner` ON `p_automations_host_maintenance`; + -- ==== plugin homepage 0001_adopt_homepage_tables.sql ==== -- homepage 0001: adopt_homepage_tables -- Generated by termix-plugin migrations. Review before committing. diff --git a/src/backend/tests/fixtures/upgrade/upgrade-postgres.sql b/src/backend/tests/fixtures/upgrade/upgrade-postgres.sql index a778c42ea..941e89fa1 100644 --- a/src/backend/tests/fixtures/upgrade/upgrade-postgres.sql +++ b/src/backend/tests/fixtures/upgrade/upgrade-postgres.sql @@ -841,6 +841,13 @@ CREATE TABLE IF NOT EXISTS "p_automations_host_maintenance" ( ); CREATE UNIQUE INDEX IF NOT EXISTS "maintenance_host_owner" ON "p_automations_host_maintenance" ("user_id", "host_id"); +-- ==== plugin automations 0004_rename_maintenance_index.sql ==== +-- automations 0004: rename_maintenance_index +-- Renames the maintenance index to the idx_ pattern every other index uses. + +CREATE UNIQUE INDEX IF NOT EXISTS "idx_automation_maintenance_host_owner" ON "p_automations_host_maintenance" ("user_id", "host_id"); +DROP INDEX IF EXISTS "maintenance_host_owner"; + -- ==== plugin homepage 0001_adopt_homepage_tables.sql ==== -- homepage 0001: adopt_homepage_tables -- Generated by termix-plugin migrations. Review before committing. diff --git a/src/backend/tests/plugins/security/data.test.ts b/src/backend/tests/plugins/security/data.test.ts index f9037d55b..62c637fce 100644 --- a/src/backend/tests/plugins/security/data.test.ts +++ b/src/backend/tests/plugins/security/data.test.ts @@ -169,14 +169,21 @@ describe("the migration checker", () => { for (const dialect of ["sqlite", "postgres", "mysql"]) { const dir = path.join(plugins, id, "migrations", dialect); if (!fs.existsSync(dir)) continue; - for (const file of fs.readdirSync(dir)) { - if (!file.endsWith(".sql")) continue; - const sql = fs.readFileSync(path.join(dir, file), "utf8"); + const files = fs + .readdirSync(dir) + .filter((file) => file.endsWith(".sql")) + .sort(); + const sqls = files.map((file) => + fs.readFileSync(path.join(dir, file), "utf8"), + ); + // The runner lets a later migration drop an index an earlier one made. + const ownedIndexes = collectOwnedIndexes(id, sqls, legacy); + files.forEach((file, index) => { expect( - findUnownedTableWrites(id, sql, legacy), + findUnownedTableWrites(id, sqls[index], legacy, ownedIndexes), `${id}/${dialect}/${file}`, ).toEqual([]); - } + }); } } });