review fixes

merged master

Author: mpcgrid

apps/webapp/app/components/logs/LogDetailView.tsx

@@ -67,17 +67,30 @@ export function LogDetailView({ logId, initialLog, onClose, searchTerm }: LogDet
   const environment = useEnvironment();
   const fetcher = useTypedFetcher<typeof logDetailLoader>();
   const [activeTab, setActiveTab] = useState<TabType>("details");
+  const [error, setError] = useState<string | null>(null);
 
   // Fetch full log details when logId changes
   useEffect(() => {
     if (!logId) return;
 
+    setError(null);
     fetcher.load(
       `/resources/orgs/${organization.slug}/projects/${project.slug}/env/${environment.slug}/logs/${encodeURIComponent(logId)}`
     );
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [organization.slug, project.slug, environment.slug, logId]);
 
+  // Handle fetch errors
+  useEffect(() => {
+    if (fetcher.data && typeof fetcher.data === "object" && "error" in fetcher.data) {
+      setError(fetcher.data.error as string);
+    } else if (fetcher.state === "idle" && fetcher.data === null && !initialLog) {
+      setError("Failed to load log details");
+    } else {
+      setError(null);
+    }
+  }, [fetcher.data, initialLog, fetcher.state]);
+
   const isLoading = fetcher.state === "loading";
   const log = fetcher.data ?? initialLog;
 
@@ -110,7 +123,7 @@ export function LogDetailView({ logId, initialLog, onClose, searchTerm }: LogDet
           </Button>
         </div>
         <div className="flex flex-1 items-center justify-center">
-          <Paragraph className="text-text-dimmed">Log not found</Paragraph>
+          <Paragraph className="text-text-dimmed">{error ?? "Log not found"}</Paragraph>
         </div>
       </div>
     );
@@ -255,20 +268,18 @@ function RunTab({ log, runPath }: { log: LogEntry; runPath: string }) {
   const project = useProject();
   const environment = useEnvironment();
   const fetcher = useTypedFetcher<RunContextData>();
-  const [requested, setRequested] = useState(false);
 
   // Fetch run details when tab is active
   useEffect(() => {
     if (!log.runId) return;
 
-    setRequested(true);
     fetcher.load(
       `/resources/orgs/${organization.slug}/projects/${project.slug}/env/${environment.slug}/logs/${encodeURIComponent(log.id)}/run?runId=${encodeURIComponent(log.runId)}`
     );
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [organization.slug, project.slug, environment.slug, log.id, log.runId]);
 
-  const isLoading = !requested || fetcher.state === "loading";
+  const isLoading = fetcher.state === "loading";
   const runData = fetcher.data?.run;
 
   if (isLoading) {
@@ -388,14 +399,12 @@ function RunTab({ log, runPath }: { log: LogEntry; runPath: string }) {
           </Property.Value>
         </Property.Item>
 
-        {environment && (
-          <Property.Item>
-            <Property.Label>Environment</Property.Label>
-            <Property.Value>
-              <EnvironmentCombo environment={environment} />
-            </Property.Value>
-          </Property.Item>
-        )}
+        <Property.Item>
+          <Property.Label>Environment</Property.Label>
+          <Property.Value>
+            <EnvironmentCombo environment={environment} />
+          </Property.Value>
+        </Property.Item>
 
         <Property.Item>
           <Property.Label>Queue</Property.Label>

apps/webapp/app/components/logs/LogsTable.tsx

@@ -5,7 +5,7 @@ import { Button } from "~/components/primitives/Buttons";
 import { useEnvironment } from "~/hooks/useEnvironment";
 import { useOrganization } from "~/hooks/useOrganizations";
 import { useProject } from "~/hooks/useProject";
-import type { LogEntry, LogsListAppliedFilters } from "~/presenters/v3/LogsListPresenter.server";
+import type { LogEntry } from "~/presenters/v3/LogsListPresenter.server";
 import { getLevelColor, highlightSearchText } from "~/utils/logUtils";
 import { v3RunSpanPath } from "~/utils/pathBuilder";
 import { DateTime } from "../primitives/DateTime";
@@ -131,7 +131,7 @@ export function LogsTable({
               {!isLoading && <NoLogs title="No logs found" />}
             </TableBlankRow>
           ) : logs.length === 0 ? (
-            <BlankState isLoading={isLoading} />
+            <BlankState isLoading={isLoading} onRefresh={() => window.location.reload()} />
           ) : (
             logs.map((log) => {
               const isSelected = selectedLogId === log.id;
@@ -222,9 +222,11 @@ function NoLogs({ title }: { title: string }) {
   );
 }
 
-function BlankState({ isLoading }: { isLoading?: boolean }) {
+function BlankState({ isLoading, onRefresh }: { isLoading?: boolean; onRefresh?: () => void }) {
   if (isLoading) return <TableBlankRow colSpan={6}></TableBlankRow>;
 
+  const handleRefresh = onRefresh ?? (() => window.location.reload());
+
   return (
     <TableBlankRow colSpan={6}>
       <div className="flex flex-col items-center justify-center gap-6">
@@ -235,9 +237,7 @@ function BlankState({ isLoading }: { isLoading?: boolean }) {
           <Button
             LeadingIcon={ArrowPathIcon}
             variant="tertiary/medium"
-            onClick={() => {
-              window.location.reload();
-            }}
+            onClick={handleRefresh}
           >
             Refresh
           </Button>

apps/webapp/app/components/runs/v3/RunFilters.tsx

@@ -326,6 +326,8 @@ type RunFiltersProps = {
   hasFilters: boolean;
   /** Hide the AI search input (useful when replacing with a custom search component) */
   hideSearch?: boolean;
+  /** Custom default period for the time filter (e.g., "1h", "7d") */
+  defaultPeriod?: string;
 };
 
 export function RunsFilters(props: RunFiltersProps) {
@@ -348,7 +350,7 @@ export function RunsFilters(props: RunFiltersProps) {
       <FilterMenu {...props} />
       {!props.hideSearch && <AIFilterInput />}
       <RootOnlyToggle defaultValue={props.rootOnlyDefault} />
-      <TimeFilter />
+      <TimeFilter defaultPeriod={props.defaultPeriod} />
       <AppliedFilters {...props} />
       {hasFilters && (
         <Form className="h-6">

apps/webapp/app/components/runs/v3/SharedFilters.tsx

@@ -260,13 +260,18 @@ export function timeFilterRenderValues({
   return { label, valueLabel, rangeType };
 }
 
-export function TimeFilter() {
+export interface TimeFilterProps {
+  defaultPeriod?: string;
+}
+
+export function TimeFilter({ defaultPeriod }: TimeFilterProps = {}) {
   const { value, del } = useSearchParams();
 
   const { period, from, to, label, valueLabel } = timeFilters({
     period: value("period"),
     from: value("from"),
     to: value("to"),
+    defaultPeriod,
   });
 
   return (
@@ -287,6 +292,7 @@ export function TimeFilter() {
           period={period}
           from={from}
           to={to}
+          defaultPeriod={defaultPeriod}
         />
       )}
     </FilterMenuProvider>

apps/webapp/app/presenters/v3/LogsListPresenter.server.ts

@@ -10,10 +10,7 @@ import {
 } from "@trigger.dev/database";
 
 // Create a schema that validates TaskRunStatus enum values
-const TaskRunStatusSchema = z.array(z.string()).refine(
-  (val) => val.every((v) => Object.values(TaskRunStatusEnum).includes(v)),
-  { message: "Invalid TaskRunStatus values" }
-) as unknown as z.ZodSchema<TaskRunStatus[]>;
+const TaskRunStatusSchema = z.array(z.nativeEnum(TaskRunStatusEnum));
 import parseDuration from "parse-duration";
 import { type Direction } from "~/components/ListPagination";
 import { timeFilters } from "~/components/runs/v3/SharedFilters";
@@ -25,7 +22,7 @@ import {
   convertDateToClickhouseDateTime,
   convertClickhouseDateTime64ToJsDate,
 } from "~/v3/eventRepository/clickhouseEventRepository.server";
-import { kindToLevel, type LogLevel } from "~/utils/logUtils";
+import { kindToLevel, type LogLevel, LogLevelSchema } from "~/utils/logUtils";
 
 export type { LogLevel };
 
@@ -56,6 +53,7 @@ export type LogsListOptions = {
   queues?: string[];
   machines?: MachinePresetName[];
   levels?: LogLevel[];
+  defaultPeriod?: string;
   // search
   search?: string;
   includeDebugLogs?: boolean;
@@ -82,8 +80,9 @@ export const LogsListOptionsSchema = z.object({
   batchId: z.string().optional(),
   runId: z.array(z.string()).optional(),
   queues: z.array(z.string()).optional(),
-  machines: z.array(z.string()).optional(),
-  levels: z.array(z.enum(["TRACE", "DEBUG", "INFO", "WARN", "ERROR", "CANCELLED"])).optional(),
+  machines: z.array(MachinePresetName).optional(),
+  levels: z.array(LogLevelSchema).optional(),
+  defaultPeriod: z.string().optional(),
   search: z.string().max(1000).optional(),
   includeDebugLogs: z.boolean().optional(),
   direction: z.enum(["forward", "backward"]).optional(),
@@ -106,14 +105,26 @@ type LogCursor = {
   runId: string;
 };
 
+const LogCursorSchema = z.object({
+  startTime: z.string(),
+  traceId: z.string(),
+  spanId: z.string(),
+  runId: z.string(),
+});
+
 function encodeCursor(cursor: LogCursor): string {
   return Buffer.from(JSON.stringify(cursor)).toString("base64");
 }
 
 function decodeCursor(cursor: string): LogCursor | null {
   try {
     const decoded = Buffer.from(cursor, "base64").toString("utf-8");
-    return JSON.parse(decoded) as LogCursor;
+    const parsed = JSON.parse(decoded);
+    const validated = LogCursorSchema.safeParse(parsed);
+    if (!validated.success) {
+      return null;
+    }
+    return validated.data;
   } catch {
     return null;
   }
@@ -189,12 +200,14 @@ export class LogsListPresenter {
       cursor,
       pageSize = DEFAULT_PAGE_SIZE,
       includeDebugLogs = true,
+      defaultPeriod,
     }: LogsListOptions
   ) {
     const time = timeFilters({
       period,
       from,
       to,
+      defaultPeriod,
     });
 
     let effectiveFrom = time.from;
@@ -418,7 +431,7 @@ export class LogsListPresenter {
 
     if (levels && levels.length > 0) {
       const conditions: string[] = [];
-      const params: Record<string, any> = {};
+      const params: Record<string, string[]> = {};
       const hasErrorOrCancelledLevel = levels.includes("ERROR") || levels.includes("CANCELLED");
 
       for (const level of levels) {
@@ -451,7 +464,7 @@ export class LogsListPresenter {
       }
 
       if (conditions.length > 0) {
-        queryBuilder.where(`(${conditions.join(" OR ")})`, params as any);
+        queryBuilder.where(`(${conditions.join(" OR ")})`, params);
       }
     }
 

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs/route.tsx

@@ -93,6 +93,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
     search,
     levels,
     includeDebugLogs: isAdmin && showDebug,
+    defaultPeriod: "1h",
   });
 
   const session = await setRootOnlyFilterPreference(filters.rootOnly, request);
@@ -105,6 +106,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
       filters,
       isAdmin,
       showDebug,
+      defaultPeriod: "1h",
     },
     {
       headers: {
@@ -115,7 +117,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
 };
 
 export default function Page() {
-  const { data, rootOnlyDefault, isAdmin, showDebug } = useTypedLoaderData<typeof loader>();
+  const { data, rootOnlyDefault, isAdmin, showDebug, defaultPeriod } = useTypedLoaderData<typeof loader>();
 
   return (
     <PageContainer>
@@ -154,6 +156,7 @@ export default function Page() {
                   rootOnlyDefault={rootOnlyDefault}
                   isAdmin={isAdmin}
                   showDebug={showDebug}
+                  defaultPeriod={defaultPeriod}
                 />
               );
             }}
@@ -169,11 +172,13 @@ function LogsList({
   rootOnlyDefault,
   isAdmin,
   showDebug,
+  defaultPeriod,
 }: {
   list: Awaited<UseDataFunctionReturn<typeof loader>["data"]>;
   rootOnlyDefault: boolean;
   isAdmin: boolean;
   showDebug: boolean;
+  defaultPeriod?: string;
 }) {
   const navigation = useNavigation();
   const location = useLocation();
@@ -284,6 +289,7 @@ function LogsList({
                 hasFilters={list.hasFilters}
                 rootOnlyDefault={rootOnlyDefault}
                 hideSearch
+                defaultPeriod={defaultPeriod}
               />
               <LogsLevelFilter showDebug={showDebug} />
               <LogsSearchInput />
@@ -302,7 +308,6 @@ function LogsList({
           <LogsTable
             logs={accumulatedLogs}
             hasFilters={list.hasFilters}
-            filters={list.filters}
             searchTerm={list.searchTerm}
             isLoading={isLoading}
             isLoadingMore={fetcher.state === "loading"}

apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.logs.ts

@@ -53,6 +53,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
     cursor,
     levels,
     includeDebugLogs: isAdmin && showDebug,
+    defaultPeriod: "1h",
   }) as any; // Validated by LogsListOptionsSchema at runtime
 
   const presenter = new LogsListPresenter($replica, clickhouseClient);

apps/webapp/app/utils/logUtils.ts

@@ -1,6 +1,10 @@
 import { createElement, Fragment, type ReactNode } from "react";
+import { z } from "zod";
 
-export type LogLevel = "TRACE" | "DEBUG" | "INFO" | "WARN" | "ERROR" | "CANCELLED";
+export const LogLevelSchema = z.enum(["TRACE", "DEBUG", "INFO", "WARN", "ERROR", "CANCELLED"]);
+export type LogLevel = z.infer<typeof LogLevelSchema>;
+
+export const validLogLevels: LogLevel[] = ["TRACE", "DEBUG", "INFO", "WARN", "ERROR", "CANCELLED"];
 
 // Default styles for search highlighting
 const DEFAULT_HIGHLIGHT_STYLES: React.CSSProperties = {
@@ -29,6 +33,12 @@ export function highlightSearchText(
     return text;
   }
 
+  // Defense in depth: limit search term length to prevent ReDoS and performance issues
+  const MAX_SEARCH_LENGTH = 500;
+  if (searchTerm.length > MAX_SEARCH_LENGTH) {
+    return text;
+  }
+
   // Escape special regex characters in search term
   const escapedSearch = searchTerm.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
   const regex = new RegExp(escapedSearch, "gi");
@@ -130,13 +140,9 @@ export function getKindLabel(kind: string): string {
     case "SPAN_EVENT":
       return "Event";
     case "LOG_DEBUG":
-      return "Log";
     case "LOG_INFO":
-      return "Log";
     case "LOG_WARN":
-      return "Log";
     case "LOG_ERROR":
-      return "Log";
     case "LOG_LOG":
       return "Log";
     case "DEBUG_EVENT":