fix: complete review hardening

This commit is contained in:
ajnart committed 2026-08-26 16:12:59 +02:00
1 parent 6f85e18065
commit 2449eb0bc7
13 files changed
+99 -64

No files matched your search

+3 -2
View File
@@ -434,9 +434,10 @@ jobs:
return;
}
const targetTags = target.metadata?.container?.tags ?? [];
if (targetTags.includes("v2")) {
const extraTags = targetTags.filter((targetTag) => targetTag !== tag);
if (extraTags.length > 0) {
core.setFailed(
`Package version ${target.id} also carries protected tag "v2"; refusing cleanup.`,
`Package version ${target.id} also carries tag(s) ${extraTags.map((targetTag) => `"${targetTag}"`).join(", ")}; refusing cleanup.`,
);
return;
}
+3 -2
View File
@@ -144,8 +144,9 @@ Docker or Podman endpoints from targeting the wrong service.
Docker environment, or pass one or more IDs returned by `docker.getEndpoints` to query only those Dockerode clients. The
Docker widget uses the same behavior for its environment selector.
`docker.refreshInventory` always refreshes the current replica and returns `scope: "all"` after cross-replica publication or
`scope: "local"` when Redis is unavailable. A local result is partial success: retry after restoring Redis to refresh every replica.
`docker.refreshInventory` always refreshes the current replica and returns `scope: "published"` when Redis accepts the
non-persistent refresh event, or `scope: "local"` when publication fails. Published scope does not confirm that every replica
processed the event. A local result is partial success: retry after restoring Redis to notify the other replicas.
## Partial integration failures
+2
View File
@@ -66,6 +66,8 @@ add_header Content-Security-Policy "frame-ancestors 'self' https://trusted-domai
- **Wikipedia homepage:** https://en.wikipedia.org/wiki/Main_Page
- **Any YouTube video or stream:** https://www.youtube.com/embed/dQw4w9WgXcQ?si=MWUbbG66UNTla_Ni
- **Same-origin page:** `/path/to/page` resolves from the current Homarr origin. Root-relative URLs are useful when Homarr and
the embedded page share a reverse-proxy host.
### Configuration
@@ -21,6 +21,9 @@ const createSuccessfulQuery = (queryKey: readonly unknown[], data: unknown, upda
return { query, queryClient };
};
const stockPriceQueryKey = (symbol = "HOMARR") =>
[["widget", "stockPrice", "getPriceHistory"], { input: { symbol }, type: "query" }] as const;
const createMemoryStorage = (): Storage => {
const values = new Map<string, string>();
return {
@@ -43,11 +46,12 @@ afterEach(() => {
describe("widget query persistence", () => {
test("persists successful dashboard data but not unrelated, ping, pending, or oversized queries", () => {
const widget = createSuccessfulQuery([["widget", "weather", "getWeather"], { type: "query" }], { temp: 21 });
const widget = createSuccessfulQuery(stockPriceQueryKey(), { price: 21 });
const location = createSuccessfulQuery([["widget", "weather", "atLocation"], { type: "query" }], { temp: 21 });
const supporting = createSuccessfulQuery([["app", "byIds"], { type: "query" }], [{ id: "app-1" }]);
const unrelated = createSuccessfulQuery([["board", "getBoardByName"], { type: "query" }], { name: "Home" });
const ping = createSuccessfulQuery([["widget", "app", "ping"], { type: "query" }], { status: "online" });
const oversized = createSuccessfulQuery([["widget", "example"], { type: "query" }], {
const oversized = createSuccessfulQuery(stockPriceQueryKey("OVERSIZED"), {
value: "x".repeat(queryPersistenceMaxDataBytes + 1),
});
const pendingClient = new QueryClient();
@@ -55,10 +59,7 @@ describe("widget query persistence", () => {
queryKey: [["widget", "calendar", "findAllEvents"], { type: "query" }],
queryFn: () => Promise.resolve([]),
});
const forbidden = createSuccessfulQuery(
[["widget", "calendar", "findAllEvents"], { type: "query" }],
[{ title: "revoked" }],
);
const forbidden = createSuccessfulQuery(stockPriceQueryKey("FORBIDDEN"), [{ title: "revoked" }]);
forbidden.query.setState({
...forbidden.query.state,
error: Object.assign(new Error("access denied"), { data: { code: "FORBIDDEN" } }),
@@ -66,7 +67,8 @@ describe("widget query persistence", () => {
});
expect(shouldPersistDashboardQuery(widget.query)).toBe(true);
expect(shouldPersistDashboardQuery(supporting.query)).toBe(true);
expect(shouldPersistDashboardQuery(location.query)).toBe(false);
expect(shouldPersistDashboardQuery(supporting.query)).toBe(false);
expect(shouldPersistDashboardQuery(unrelated.query)).toBe(false);
expect(shouldPersistDashboardQuery(ping.query)).toBe(false);
expect(shouldPersistDashboardQuery(oversized.query)).toBe(false);
@@ -100,7 +102,7 @@ describe("widget query persistence", () => {
});
test("never restores another session scope", async () => {
const queryKey = [["widget", "weather", "getWeather"], { type: "query" }] as const;
const queryKey = stockPriceQueryKey();
const { queryClient } = createSuccessfulQuery(queryKey, { temp: 21 });
const userAPersister = createSessionQueryPersister("user-a");
userAPersister.persistClient({
@@ -117,9 +119,7 @@ describe("widget query persistence", () => {
test("stops persisting once the provider tears the cache down", async () => {
const storage = createMemoryStorage();
const persister = createSessionQueryPersister("user-a", storage);
const { queryClient } = createSuccessfulQuery([["widget", "weather", "getWeather"], { type: "query" }], {
temp: 21,
});
const { queryClient } = createSuccessfulQuery(stockPriceQueryKey(), { price: 21 });
persister.persistClient({
timestamp: Date.now(),
@@ -160,7 +160,7 @@ describe("widget query persistence", () => {
const scope = "user-a";
const key = getQueryPersistenceStorageKey(scope);
const persister = createSessionQueryPersister(scope);
const { query, queryClient } = createSuccessfulQuery([["widget", "calendar", "findAllEvents"], { type: "query" }], {
const { query, queryClient } = createSuccessfulQuery(stockPriceQueryKey(), {
startsAt: new Date("2026-08-22T12:00:00.000Z"),
});
query.setState({ ...query.state, error: new Error("private refresh error"), status: "error" });
@@ -202,10 +202,7 @@ describe("widget query persistence", () => {
test("sanitizes structurally valid but unsafe stored state before hydration", async () => {
const scope = "user-a";
const key = getQueryPersistenceStorageKey(scope);
const { queryClient } = createSuccessfulQuery(
[["widget", "calendar", "findAllEvents"], { type: "query" }],
[{ title: "safe" }],
);
const { queryClient } = createSuccessfulQuery(stockPriceQueryKey(), [{ title: "safe" }]);
const validQuery = dehydrate(queryClient).queries[0];
if (!validQuery) throw new Error("Expected a dehydrated query");
@@ -259,9 +256,7 @@ describe("widget query persistence", () => {
vi.setSystemTime(new Date("2026-08-22T12:00:00.000Z"));
const scope = "user-a";
const persister = createSessionQueryPersister(scope);
const { queryClient } = createSuccessfulQuery([["widget", "weather", "getWeather"], { type: "query" }], {
temp: 21,
});
const { queryClient } = createSuccessfulQuery(stockPriceQueryKey(), { price: 21 });
persister.persistClient({ timestamp, buster: storedBuster, clientState: dehydrate(queryClient) });
persister.flush();
@@ -273,13 +268,13 @@ describe("widget query persistence", () => {
buster: queryPersistenceBuster,
});
expect(restoredClient.getQueryData([["widget", "weather", "getWeather"], { type: "query" }])).toBeUndefined();
expect(restoredClient.getQueryData(stockPriceQueryKey())).toBeUndefined();
expect(window.sessionStorage.getItem(getQueryPersistenceStorageKey(scope))).toBeNull();
});
test("quota retry writes the cache after evicting the oldest query", async () => {
const old = createSuccessfulQuery([["widget", "old"], { type: "query" }], "old", 1);
const recent = createSuccessfulQuery([["widget", "recent"], { type: "query" }], "recent", 2);
const old = createSuccessfulQuery(stockPriceQueryKey("OLD"), "old", 1);
const recent = createSuccessfulQuery(stockPriceQueryKey("RECENT"), "recent", 2);
const storage = createMemoryStorage();
const setItem = storage.setItem.bind(storage);
const setItemSpy = vi.spyOn(storage, "setItem").mockImplementation((key, value) => {
@@ -303,7 +298,7 @@ describe("widget query persistence", () => {
expect(setItemSpy).toHaveBeenCalledTimes(2);
const restored = await persister.restoreClient();
expect(restored?.clientState.queries).toHaveLength(1);
expect(restored?.clientState.queries[0]?.queryKey).toEqual([["widget", "recent"], { type: "query" }]);
expect(restored?.clientState.queries[0]?.queryKey).toEqual(stockPriceQueryKey("RECENT"));
persister.persistClient(persistedClient);
persister.flush();
@@ -314,7 +309,7 @@ describe("widget query persistence", () => {
vi.useFakeTimers();
const storage = createMemoryStorage();
const setItem = vi.spyOn(storage, "setItem");
const queryKey = [["widget", "weather", "getWeather"], { type: "query" }] as const;
const queryKey = stockPriceQueryKey();
const first = createSuccessfulQuery(queryKey, { temp: 20 }, 1);
const latest = createSuccessfulQuery(queryKey, { temp: 21 }, 2);
const persister = createSessionQueryPersister("user-a", storage);
@@ -355,7 +350,7 @@ describe("widget query persistence", () => {
},
} as unknown as Storage;
const persister = createSessionQueryPersister("blocked", blockedStorage);
const { queryClient } = createSuccessfulQuery([["widget", "weather"], { type: "query" }], { temp: 21 });
const { queryClient } = createSuccessfulQuery(stockPriceQueryKey(), { price: 21 });
expect(persister.restoreClient()).toBeUndefined();
expect(() => {
@@ -2,14 +2,18 @@ import type { QueryKey } from "@tanstack/react-query";
import type { PersistedClient, Persister } from "@tanstack/react-query-persist-client";
import { parse, stringify } from "superjson";
import { isWidgetDataQueryKey, queryCacheDefaultGcTimeMs } from "@homarr/api/query-cache";
import {
isPersistableDashboardQueryKey,
isTrpcForbiddenError,
queryCacheDefaultGcTimeMs,
} from "@homarr/api/query-cache";
export const queryPersistenceMaxAgeMs = queryCacheDefaultGcTimeMs;
export const queryPersistenceMaxDataBytes = 256 * 1024;
export const queryPersistenceMaxCacheBytes = 2 * 1024 * 1024;
export const queryPersistenceThrottleMs = 1_000;
export const queryPersistenceIdleTimeoutMs = 1_000;
export const queryPersistenceBuster = "v3-forbidden-filter";
export const queryPersistenceBuster = "v4-public-data-only";
const queryPersistenceStoragePrefix = "homarr:widget-query-cache";
@@ -33,15 +37,6 @@ const budgetVerdictCache = new WeakMap<object, boolean>();
const isRecord = (value: unknown): value is Record<string, unknown> =>
typeof value === "object" && value !== null && !Array.isArray(value);
interface ForbiddenTrpcError {
data: { code: "FORBIDDEN" };
}
const isTrpcForbiddenError = (error: unknown): error is ForbiddenTrpcError => {
if (!isRecord(error) || !isRecord(error.data)) return false;
return error.data.code === "FORBIDDEN";
};
const isQuotaExceededError = (error: unknown): boolean => {
if (!isRecord(error)) return false;
return (
@@ -77,7 +72,7 @@ const holdsDashboardData = (query: PersistableQuery) =>
query.state.data !== undefined &&
Number.isFinite(query.state.dataUpdatedAt) &&
query.state.dataUpdatedAt > 0 &&
isWidgetDataQueryKey(query.queryKey);
isPersistableDashboardQueryKey(query.queryKey);
export const shouldPersistDashboardQuery = (query: PersistableQuery) =>
holdsDashboardData(query) && !isTrpcForbiddenError(query.state.error) && fitsStorageBudget(query.state.data);
@@ -119,7 +114,7 @@ const sanitizePersistedClient = (client: PersistedClient): PersistedClient => ({
clientState: {
mutations: [],
queries: client.clientState.queries
.filter((query) => !isTrpcForbiddenError(query.state.error))
.filter((query) => holdsDashboardData(query) && !isTrpcForbiddenError(query.state.error))
.map(sanitizePersistedQuery),
},
});
@@ -268,10 +263,9 @@ export const createSessionQueryPersister = (
}
quotaLimited = true;
candidate = keepNewestQueries(candidate, Math.floor(candidate.clientState.queries.length / 2));
if (!candidate) return;
}
}
persistenceDisabled = true;
};
const clearScheduledPersist = () => {
@@ -3,7 +3,7 @@
import type { PropsWithChildren } from "react";
import { useCallback, useEffect, useMemo, useState } from "react";
import dynamic from "next/dynamic";
import { QueryClient } from "@tanstack/react-query";
import { QueryCache, QueryClient } from "@tanstack/react-query";
import { ReactQueryStreamedHydration } from "@tanstack/react-query-next-experimental";
import { PersistQueryClientProvider } from "@tanstack/react-query-persist-client";
import {
@@ -25,6 +25,8 @@ import type { AppRouter } from "@homarr/api";
import { clientApi } from "@homarr/api/client";
import {
dashboardSupportingQueryPolicies,
isTrpcForbiddenError,
isWidgetDataQueryKey,
queryCacheDefaultGcTimeMs,
queryCacheDefaultRefetchIntervalMs,
queryCacheDefaultStaleTimeMs,
@@ -114,6 +116,12 @@ const ScopedTRPCReactProvider = ({
);
const [queryClient] = useState(() => {
const client = new QueryClient({
queryCache: new QueryCache({
onError(error, query) {
if (!isTrpcForbiddenError(error) || !isWidgetDataQueryKey(query.queryKey)) return;
query.setState({ data: undefined, dataUpdatedAt: 0, isInvalidated: true });
},
}),
defaultOptions: {
queries: {
staleTime: queryCacheDefaultStaleTimeMs,
@@ -41,7 +41,7 @@ import type { UrlTemplateMode } from "@homarr/definitions";
import { getIntegrationName, invariantTechnicalLabels } from "@homarr/definitions";
import { useModalAction } from "@homarr/modals";
import { AddDockerAppToHomarr } from "@homarr/modals-collection";
import { showErrorNotification } from "@homarr/notifications";
import { showErrorNotification, showWarningNotification } from "@homarr/notifications";
import { normalizeServiceUrl, ServiceUrlTemplate } from "@homarr/onboarding";
import { useI18n } from "@homarr/translation/client";
import { Link } from "@homarr/ui";
@@ -69,12 +69,18 @@ export const DockerReconciliation = ({ defaultServerOrigin }: { defaultServerOri
const reconciliation = clientApi.docker.reconcileServices.useQuery();
const health = clientApi.docker.getServiceHealth.useQuery(undefined, { enabled: isOpen });
const refreshInventory = clientApi.docker.refreshInventory.useMutation({
async onSuccess() {
async onSuccess(result) {
await Promise.all([
utils.docker.getContainers.invalidate(),
utils.docker.reconcileServices.invalidate(),
utils.docker.getServiceHealth.invalidate(),
]);
if (result.scope === "local") {
showWarningNotification({
title: tDockerAction("refresh.notification.warning.title"),
message: tDockerAction("refresh.notification.warning.message"),
});
}
},
onError() {
showErrorNotification({
@@ -25,7 +25,7 @@ import type { ContainerState, DockerEndpointCapability } from "@homarr/docker";
import { containerStateColorMap, cpuUsageColor, memoryUsageColor, safeValue } from "@homarr/docker/shared";
import { useModalAction } from "@homarr/modals";
import { AddDockerAppToHomarr, useDockerContainerRemovalConfirmation } from "@homarr/modals-collection";
import { showErrorNotification, showSuccessNotification } from "@homarr/notifications";
import { showErrorNotification, showSuccessNotification, showWarningNotification } from "@homarr/notifications";
import type { ScopedTranslationFunction } from "@homarr/translation";
import { useI18n } from "@homarr/translation/client";
import type { TablerIcon } from "@homarr/ui";
@@ -205,7 +205,13 @@ export function DockerTable({ initialData }: DockerTableProps) {
utils.docker.reconcileServices.invalidate(),
utils.docker.getServiceHealth.invalidate(),
]);
if (result.scope === "local") return;
if (result.scope === "local") {
showWarningNotification({
title: tDocker("action.refresh.notification.warning.title"),
message: tDocker("action.refresh.notification.warning.message"),
});
return;
}
showSuccessNotification({
title: tDocker("action.refresh.notification.success.title"),
message: tDocker("action.refresh.notification.success.message"),
+21
View File
@@ -1,5 +1,16 @@
import type { QueryKey } from "@tanstack/react-query";
const isRecord = (value: unknown): value is Record<string, unknown> => typeof value === "object" && value !== null;
interface ForbiddenTrpcError {
data: { code: "FORBIDDEN" };
}
export const isTrpcForbiddenError = (error: unknown): error is ForbiddenTrpcError => {
if (!isRecord(error) || !isRecord(error.data)) return false;
return error.data.code === "FORBIDDEN";
};
// RSC-prefetched data must survive the hydration pass without an immediate
// duplicate fetch. Widget-specific intervals still control live refreshes.
export const queryCacheDefaultStaleTimeMs = 30_000;
@@ -8,6 +19,7 @@ export const queryCacheDefaultGcTimeMs = 1000 * 60 * 5;
export const queryCacheMetadataStaleTimeMs = 1000 * 60 * 5;
const widgetDataQueryPaths = new Set(["app.byId", "app.byIds", "docker.getContainers", "integration.byIds"]);
const persistableDashboardDataPaths = new Set(["widget.stockPrice.getPriceHistory"]);
// Beszel system stats are large time-series payloads and were deliberately kept out of
// the persisted cache in #6289; live stats come from an SSE subscription instead.
const excludedWidgetPaths = new Set(["widget.app.ping", "widget.beszel.getSystemStats"]);
@@ -66,3 +78,12 @@ export const isWidgetDataQueryKey = (queryKey: QueryKey) => {
return isWidgetDataTrpcPath(path.join("."));
};
// Persist only data that is both public and independent of Homarr access grants.
// Integration, app, Docker, board-backed, and location queries must refetch after reload.
export const isPersistableDashboardQueryKey = (queryKey: QueryKey) => {
const path = getTrpcPathFromQueryKey(queryKey);
if (!path) return false;
return persistableDashboardDataPaths.has(path.join("."));
};
@@ -87,14 +87,14 @@ export const dockerRouter = createTRPCRouter({
mcp: {
enabled: true,
description:
"Invalidate the cached Docker inventory so the next Docker query performs a fresh user-requested discovery. Requires admin permission.",
"Invalidate the local Docker inventory and publish a non-persistent Redis refresh event. Returns scope 'published' when Redis accepts the event or 'local' when only this replica is refreshed. Requires admin permission.",
},
})
.concat(dockerMiddleware())
.mutation(async () => {
resetDockerInventory();
const propagated = await publishDockerInventoryRefreshAsync();
if (propagated) return { scope: "all" as const };
if (propagated) return { scope: "published" as const };
return { scope: "local" as const };
}),
getContainers: permissionRequiredProcedure
+4
View File
@@ -6644,6 +6644,10 @@
"title": "Containers refreshed",
"message": "You are now viewing the most recent data"
},
"warning": {
"title": "Only this Homarr instance was refreshed",
"message": "Redis did not accept the refresh event, so other Homarr instances may still show older Docker data"
},
"error": {
"title": "Containers not refreshed",
"message": "Something went wrong while refreshing the containers"
+2 -11
View File
@@ -1,3 +1,5 @@
import { isTrpcForbiddenError } from "@homarr/api/query-cache";
interface WidgetQueryState {
data: unknown;
error: unknown;
@@ -7,17 +9,6 @@ interface PendingWidgetQueryState extends WidgetQueryState {
isPending: boolean;
}
const isRecord = (value: unknown): value is Record<string, unknown> => typeof value === "object" && value !== null;
interface ForbiddenTrpcError {
data: { code: "FORBIDDEN" };
}
export const isTrpcForbiddenError = (error: unknown): error is ForbiddenTrpcError => {
if (!isRecord(error) || !isRecord(error.data)) return false;
return error.data.code === "FORBIDDEN";
};
/** Throws terminal query failures while preserving successful empty values and stale cached data. */
export const getUsableWidgetQueryData = <TQuery extends WidgetQueryState>(query: TQuery): TQuery["data"] => {
if (isTrpcForbiddenError(query.error)) throw query.error;
+8 -2
View File
@@ -42,7 +42,7 @@ import type { ContainerState, DockerEndpointCapability } from "@homarr/docker";
import { containerStateColorMap, cpuUsageColor, memoryUsageColor, safeValue } from "@homarr/docker/shared";
import { useModalAction } from "@homarr/modals";
import { AddDockerAppToHomarr, useDockerContainerRemovalConfirmation } from "@homarr/modals-collection";
import { showErrorNotification, showSuccessNotification } from "@homarr/notifications";
import { showErrorNotification, showSuccessNotification, showWarningNotification } from "@homarr/notifications";
import { useI18n } from "@homarr/translation/client";
import { zoomCompensatedSize } from "@homarr/ui";
@@ -285,12 +285,18 @@ export default function DockerWidget({
const data = getUsableWidgetQueryData(containersQuery);
const { isFetching } = containersQuery;
const refreshInventory = clientApi.docker.refreshInventory.useMutation({
async onSuccess() {
async onSuccess(result) {
await Promise.all([
utils.docker.getContainers.invalidate(),
utils.docker.reconcileServices.invalidate(),
utils.docker.getServiceHealth.invalidate(),
]);
if (result.scope === "local") {
showWarningNotification({
title: t("action.refresh.notification.warning.title"),
message: t("action.refresh.notification.warning.message"),
});
}
},
onError() {
showErrorNotification({