Skip to content

Conversation

@jp-agenta
Copy link
Member

Chore/swap sendgrid by smtp

Copilot AI review requested due to automatic review settings November 11, 2025 17:33
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


GitHub CI seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR performs a major refactoring to swap email provider from SendGrid to SMTP and restructures several core modules, including:

  • Removal of legacy applications, queries, and several evaluation-related tables
  • Simplification of authentication logic by removing PostHog feature flag integrations
  • Consolidation of database migration utilities from async to sync operations
  • Renaming of evaluation entities (Results→Steps, Metrics→Metric)
  • Removal of extensive test files and manual evaluation SDK examples
  • Addition of new utility files and interface definitions

Reviewed Changes

Copilot reviewed 116 out of 2328 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
api/oss/src/apis/fastapi/evaluations/models.py Renamed evaluation entities and updated field names
api/oss/src/apis/fastapi/applications/utils.py Removed entire file containing application revision parsing utilities
api/oss/src/apis/fastapi/applications/router.py Removed entire legacy applications router
api/oss/src/apis/fastapi/applications/models.py Removed entire file with application API models
api/oss/src/apis/fastapi/annotations/utils.py Added new utility functions for annotations
api/oss/src/apis/fastapi/annotations/router.py Major rewrite of annotations router with new implementation
api/oss/src/apis/fastapi/annotations/models.py Restructured annotation models with new enums
api/oss/src/__init__.py Simplified authentication by removing PostHog integration
api/oss/docker/Dockerfile.gh Removed SDK installation and cron job setup
api/oss/docker/Dockerfile.dev Removed PYTHONPATH setup and cron job configuration
api/oss/databases/postgres/migrations/utils.py Converted async functions to sync for database operations
Multiple migration files Removed several database migration files and updated references
api/entrypoint.py Reorganized imports and router initialization
Multiple EE test files Removed manual evaluation SDK test files
Multiple EE service files Updated import statements and function signatures

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +102 to +103
span_type_enum = sa.Enum(SpanType, name="tracetype")
trace_type_enum = sa.Enum(TraceType, name="spantype")
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The enum names are swapped. span_type_enum should use name='spantype' and trace_type_enum should use name='tracetype'. This will cause the wrong enum types to be dropped during downgrade.

Suggested change
span_type_enum = sa.Enum(SpanType, name="tracetype")
trace_type_enum = sa.Enum(TraceType, name="spantype")
span_type_enum = sa.Enum(SpanType, name="spantype")
trace_type_enum = sa.Enum(TraceType, name="tracetype")

Copilot uses AI. Check for mistakes.
).scalar()

if (count or 0) > 0:
if count > 0:
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition should check if (count or 0) > 0: since count could be None. The removed version handled this case, but the new version doesn't.

Suggested change
if count > 0:
if (count or 0) > 0:

Copilot uses AI. Check for mistakes.
status_code=409,
detail="User is already a member of the workspace",
)
raise Exception("User is already a member of the workspace")
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed from HTTPException with status 409 to generic Exception. This breaks the API contract and removes proper HTTP error handling. Should remain as HTTPException.

Copilot uses AI. Check for mistakes.
"Click the link below to accept the invitation:</p><br>"
f'<a href="{invite_link}">Accept Invitation</a>'
),
call_to_action=f'Click the link below to accept the invitation:</p><br><a href="{env.AGENTA_WEB_URL}/auth?token={token}&email={email}&org_id={organization.id}&workspace_id={workspace.id}&project_id={project_id}">Accept Invitation</a>',
Copy link

Copilot AI Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

URL parameters are no longer URL-encoded. The email and token parameters should be URL-encoded to prevent injection issues and handle special characters properly.

Copilot uses AI. Check for mistakes.
Comment on lines +75 to +77
const runRes = await axios.get(
`/preview/evaluations/runs/${evaluationTableId}?project_id=${projectId}`,
)

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
of this request depends on a
user-provided value
.

Copilot Autofix

AI about 1 month ago

The best way to fix the issue is to validate the evaluationTableId parameter so that only permitted (safe) values can be used as part of the outgoing request URL. Since the parameter is used in the URL path, prevent path traversal and malicious input by enforcing that it matches the expected format (e.g., alphanumeric, UUID, typical database id). In this context, a simple validation function (using a regex or type check) should be used to ensure only valid run IDs are accepted. If validation fails, the function should avoid making the request and return an error or null instead.
This change should be applied in the useEvaluationRunData hook, in the file web/ee/src/lib/hooks/useEvaluationRunData/index.ts, intercepting and validating evaluationTableId before passing it to the request at line 76.
This will require adding a helper function for validation (for example, a simple regex test for UUID or allowed characters), and updating the code to use it before the request.

Suggested changeset 1
web/ee/src/lib/hooks/useEvaluationRunData/index.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/web/ee/src/lib/hooks/useEvaluationRunData/index.ts b/web/ee/src/lib/hooks/useEvaluationRunData/index.ts
--- a/web/ee/src/lib/hooks/useEvaluationRunData/index.ts
+++ b/web/ee/src/lib/hooks/useEvaluationRunData/index.ts
@@ -67,11 +67,31 @@
     const enrichRun = useEnrichEvaluationRun({debug})
 
     // New fetcher for preview runs that fetches and enriches with testsetData
+    // Validate evaluationTableId - allow only safe alphanumeric/uuid
+    function isValidEvaluationId(id: string | null): boolean {
+        // Accept UUIDs or alphanumeric ids (customize if needed)
+        if (!id) return false;
+        // UUIDv4 pattern or 24-char hex, or strict alphanumeric (adjust if needed)
+        const uuidRegex = /^[0-9a-fA-F]{24}$|^[0-9a-fA-F\-]{36}$/;
+        const alnumRegex = /^[a-zA-Z0-9_\-]+$/;
+        return uuidRegex.test(id) || alnumRegex.test(id);
+    }
     const fetchAndEnrichPreviewRun = useCallback(async () => {
         evalAtomStore().set(loadingStateAtom, (draft) => {
             draft.isLoadingEvaluation = true
             draft.activeStep = "eval-run"
         })
+        if (!isValidEvaluationId(evaluationTableId)) {
+            console.error("[useEvaluationRunData] Invalid evaluationTableId:", evaluationTableId)
+            evalAtomStore().set(loadingStateAtom, (draft) => {
+                draft.isLoadingEvaluation = false
+                draft.activeStep = null
+            })
+            evalAtomStore().set(evaluationRunStateAtom, (draft) => {
+                draft.isPreview = false
+            })
+            return null
+        }
         const runRes = await axios.get(
             `/preview/evaluations/runs/${evaluationTableId}?project_id=${projectId}`,
         )
EOF
@@ -67,11 +67,31 @@
const enrichRun = useEnrichEvaluationRun({debug})

// New fetcher for preview runs that fetches and enriches with testsetData
// Validate evaluationTableId - allow only safe alphanumeric/uuid
function isValidEvaluationId(id: string | null): boolean {
// Accept UUIDs or alphanumeric ids (customize if needed)
if (!id) return false;
// UUIDv4 pattern or 24-char hex, or strict alphanumeric (adjust if needed)
const uuidRegex = /^[0-9a-fA-F]{24}$|^[0-9a-fA-F\-]{36}$/;
const alnumRegex = /^[a-zA-Z0-9_\-]+$/;
return uuidRegex.test(id) || alnumRegex.test(id);
}
const fetchAndEnrichPreviewRun = useCallback(async () => {
evalAtomStore().set(loadingStateAtom, (draft) => {
draft.isLoadingEvaluation = true
draft.activeStep = "eval-run"
})
if (!isValidEvaluationId(evaluationTableId)) {
console.error("[useEvaluationRunData] Invalid evaluationTableId:", evaluationTableId)
evalAtomStore().set(loadingStateAtom, (draft) => {
draft.isLoadingEvaluation = false
draft.activeStep = null
})
evalAtomStore().set(evaluationRunStateAtom, (draft) => {
draft.isPreview = false
})
return null
}
const runRes = await axios.get(
`/preview/evaluations/runs/${evaluationTableId}?project_id=${projectId}`,
)
Copilot is powered by AI and may make mistakes. Always verify output.
export const fetchEvaluation = async (evaluationId: string) => {
const {projectId} = getCurrentProject()

const response = await axios.get(`/evaluations/${evaluationId}?project_id=${projectId}`)

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
of this request depends on a
user-provided value
.
The
URL
of this request depends on a
user-provided value
.

Copilot Autofix

AI about 1 month ago

General fix:
Validate and sanitize all values originating from user input (in this case evaluationId and, by implication, evaluationIds in array form) before using them to construct outbound request URLs, particularly where the path or query parameters are involved.

Best specific fix:
Since evaluationId is coming from the query parameters and may be an array (or a string), we should validate that it is a legitimate identifier. If, in the system, evaluationId is always expected to be a UUID (v4), use a regular expression (or npm package) to check for a UUID format before proceeding. If invalid, throw an error or ignore the request.
The validation should be performed at the entry point—in this case, in the consumer file (EvaluationCompare.tsx), where we parse and use the query. We should filter the evaluationIds to only include valid UUIDs, so that only these are ever passed down to the API calls. This prevents any malformed or malicious input from reaching the API layer.

What to change:

  • In EvaluationCompare.tsx, filter and validate evaluationIds before passing them to fetchAllComparisonResults.
  • Optionally, validate/sanitize again in the API service layer as a defense-in-depth measure (optional since we cannot change the backend and may only edit the frontend here).

Required additions:

  • Utility function for UUID validation (import one or add a regex).
  • Replace assignments to evaluationIdsArray and/or evaluationIds in EvaluationCompare.tsx with validated arrays.

Suggested changeset 1
web/ee/src/components/pages/evaluations/evaluationCompare/EvaluationCompare.tsx
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/web/ee/src/components/pages/evaluations/evaluationCompare/EvaluationCompare.tsx b/web/ee/src/components/pages/evaluations/evaluationCompare/EvaluationCompare.tsx
--- a/web/ee/src/components/pages/evaluations/evaluationCompare/EvaluationCompare.tsx
+++ b/web/ee/src/components/pages/evaluations/evaluationCompare/EvaluationCompare.tsx
@@ -8,6 +8,10 @@
 import Link from "next/link"
 import {createUseStyles} from "react-jss"
 
+// UUIDv4 validation regex from RFC 4122
+function isValidUUIDv4(id: string): boolean {
+    return /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test(id)
+}
 import AgCustomHeader from "@/oss/components/AgCustomHeader/AgCustomHeader"
 import CompareOutputDiff from "@/oss/components/CompareOutputDiff/CompareOutputDiff"
 import {useAppTheme} from "@/oss/components/Layout/ThemeContextProvider"
@@ -75,7 +79,10 @@
     const classes = useStyles()
     const {appTheme} = useAppTheme()
     const [evaluationIdsStr = ""] = useQueryParam("evaluations")
-    const evaluationIdsArray = evaluationIdsStr.split(",").filter((item) => !!item)
+    const evaluationIdsArray = evaluationIdsStr
+        .split(",")
+        .map((item) => item.trim())
+        .filter((item) => isValidUUIDv4(item))
     const [evalIds, setEvalIds] = useState(evaluationIdsArray)
     const [hiddenVariants, setHiddenVariants] = useState<string[]>([])
     const [fetching, setFetching] = useState(false)
@@ -122,7 +129,11 @@
     }, [variants])
 
     const evaluationIds = useMemo(
-        () => evaluationIdsStr.split(",").filter((item) => !!item),
+        () =>
+            evaluationIdsStr
+                .split(",")
+                .map((item) => item.trim())
+                .filter((item) => isValidUUIDv4(item)),
         [evaluationIdsStr],
     )
 
EOF
@@ -8,6 +8,10 @@
import Link from "next/link"
import {createUseStyles} from "react-jss"

// UUIDv4 validation regex from RFC 4122
function isValidUUIDv4(id: string): boolean {
return /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i.test(id)
}
import AgCustomHeader from "@/oss/components/AgCustomHeader/AgCustomHeader"
import CompareOutputDiff from "@/oss/components/CompareOutputDiff/CompareOutputDiff"
import {useAppTheme} from "@/oss/components/Layout/ThemeContextProvider"
@@ -75,7 +79,10 @@
const classes = useStyles()
const {appTheme} = useAppTheme()
const [evaluationIdsStr = ""] = useQueryParam("evaluations")
const evaluationIdsArray = evaluationIdsStr.split(",").filter((item) => !!item)
const evaluationIdsArray = evaluationIdsStr
.split(",")
.map((item) => item.trim())
.filter((item) => isValidUUIDv4(item))
const [evalIds, setEvalIds] = useState(evaluationIdsArray)
const [hiddenVariants, setHiddenVariants] = useState<string[]>([])
const [fetching, setFetching] = useState(false)
@@ -122,7 +129,11 @@
}, [variants])

const evaluationIds = useMemo(
() => evaluationIdsStr.split(",").filter((item) => !!item),
() =>
evaluationIdsStr
.split(",")
.map((item) => item.trim())
.filter((item) => isValidUUIDv4(item)),
[evaluationIdsStr],
)

Copilot is powered by AI and may make mistakes. Always verify output.
const {projectId} = getCurrentProject()

const [{data: evaluationScenarios}, evaluation] = await Promise.all([
axios.get(`/evaluations/${evaluationId}/evaluation_scenarios?project_id=${projectId}`),

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
of this request depends on a
user-provided value
.
The
URL
of this request depends on a
user-provided value
.

Copilot Autofix

AI about 1 month ago

General fix:
Sanitize and validate all user-controlled input before using it in constructing URLs for backend requests, especially where these control resource ids or path fragments.

Detailed fix:

  • Before passing evaluationIds into fetchAllComparisonResults, ensure every item in the array is both present and matches the expected format (likely a UUID – as per the application context).
  • In fetchAllEvaluationScenarios, strongly validate evaluationId before interpolating it into the URL.
  • Ideally, filter the array to include only properly formatted UUIDs, or throw an error if any id is invalid.

Changes required:

  • In web/ee/src/services/evaluations/api/index.ts, add a function to check that a string is a valid UUID (v4).
  • In fetchAllComparisonResults, validate/filter all evaluationIds with this function before proceeding.
  • Alternatively (or also), validate evaluationId at the start of fetchAllEvaluationScenarios and throw if invalid.
  • Import a commonly-used UUID validation library, e.g., validator's isUUID, or implement a regex-based validator for UUID v4.

Suggested changeset 1
web/ee/src/services/evaluations/api/index.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/web/ee/src/services/evaluations/api/index.ts b/web/ee/src/services/evaluations/api/index.ts
--- a/web/ee/src/services/evaluations/api/index.ts
+++ b/web/ee/src/services/evaluations/api/index.ts
@@ -1,5 +1,5 @@
 import uniqBy from "lodash/uniqBy"
-import {v4 as uuidv4} from "uuid"
+import {v4 as uuidv4, validate as validateUuid, version as uuidVersion} from "uuid"
 
 import {getCurrentProject} from "@/oss/contexts/project.context"
 import axios from "@/oss/lib/api/assets/axiosConfig"
@@ -196,6 +196,9 @@
 
 // Evaluation Scenarios
 export const fetchAllEvaluationScenarios = async (evaluationId: string) => {
+    if (!isValidUuidV4(evaluationId)) {
+        throw new Error("Invalid evaluationId")
+    }
     const {projectId} = getCurrentProject()
 
     const [{data: evaluationScenarios}, evaluation] = await Promise.all([
@@ -223,8 +226,18 @@
 }
 
 // Comparison
+// Helper to check UUIDv4
+function isValidUuidV4(id: string): boolean {
+    return validateUuid(id) && uuidVersion(id) === 4;
+}
+
 export const fetchAllComparisonResults = async (evaluationIds: string[]) => {
-    const scenarioGroups = await Promise.all(evaluationIds.map(fetchAllEvaluationScenarios))
+    // Only proceed with valid UUID v4 ids
+    const filteredIds = evaluationIds.filter(isValidUuidV4)
+    if (filteredIds.length !== evaluationIds.length) {
+        throw new Error('One or more evaluationIds are invalid')
+    }
+    const scenarioGroups = await Promise.all(filteredIds.map(fetchAllEvaluationScenarios))
     const testset: TestSet = await fetchTestset(scenarioGroups[0][0].evaluation?.testset?.id)
 
     const inputsNameSet = new Set<string>()
EOF
@@ -1,5 +1,5 @@
import uniqBy from "lodash/uniqBy"
import {v4 as uuidv4} from "uuid"
import {v4 as uuidv4, validate as validateUuid, version as uuidVersion} from "uuid"

import {getCurrentProject} from "@/oss/contexts/project.context"
import axios from "@/oss/lib/api/assets/axiosConfig"
@@ -196,6 +196,9 @@

// Evaluation Scenarios
export const fetchAllEvaluationScenarios = async (evaluationId: string) => {
if (!isValidUuidV4(evaluationId)) {
throw new Error("Invalid evaluationId")
}
const {projectId} = getCurrentProject()

const [{data: evaluationScenarios}, evaluation] = await Promise.all([
@@ -223,8 +226,18 @@
}

// Comparison
// Helper to check UUIDv4
function isValidUuidV4(id: string): boolean {
return validateUuid(id) && uuidVersion(id) === 4;
}

export const fetchAllComparisonResults = async (evaluationIds: string[]) => {
const scenarioGroups = await Promise.all(evaluationIds.map(fetchAllEvaluationScenarios))
// Only proceed with valid UUID v4 ids
const filteredIds = evaluationIds.filter(isValidUuidV4)
if (filteredIds.length !== evaluationIds.length) {
throw new Error('One or more evaluationIds are invalid')
}
const scenarioGroups = await Promise.all(filteredIds.map(fetchAllEvaluationScenarios))
const testset: TestSet = await fetchTestset(scenarioGroups[0][0].evaluation?.testset?.id)

const inputsNameSet = new Set<string>()
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +18 to +23
const res = await fetch(
`${apiUrl}/preview/evaluations/scenarios/${scenarioId}?project_id=${projectId}`,
{
headers: {Authorization: `Bearer ${jwt}`},
},
)

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
of this request depends on a
user-provided value
.

Copilot Autofix

AI about 1 month ago

The best way to fix this SSRF vulnerability is to strictly validate scenarioId before using it as a path segment of an outgoing HTTP request. Rather than using the raw, untrusted query parameter, we should ensure that scenarioId is present in a list of known safe scenario IDs—these should be IDs obtained from the backend (e.g., as part of the evaluation run's permitted scenario IDs), not directly from user input. This means in SingleScenarioViewer, when resolving the scenario ID from router query, we must validate that it is among the permissible scenario IDs. Only allow it to propagate further if it's safe. Otherwise, select a default safe scenarioId (such as the first scenario ID returned from the backend).

The only changes needed are in the block of code from SingleScenarioViewer (file: web/ee/src/components/EvalRunDetails/components/SingleScenarioViewer/index.tsx), specifically in the way activeId and currentScenarioId are resolved from router query. We should update the code so that when determining activeId, we pick from the intersection of the loaded, valid scenarioIds.

No library imports are needed, as simple array inclusion suffices.


Suggested changeset 1
web/ee/src/components/EvalRunDetails/components/SingleScenarioViewer/index.tsx
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/web/ee/src/components/EvalRunDetails/components/SingleScenarioViewer/index.tsx b/web/ee/src/components/EvalRunDetails/components/SingleScenarioViewer/index.tsx
--- a/web/ee/src/components/EvalRunDetails/components/SingleScenarioViewer/index.tsx
+++ b/web/ee/src/components/EvalRunDetails/components/SingleScenarioViewer/index.tsx
@@ -32,18 +32,28 @@
     const setUrlState = useSetAtom(setUrlStateAtom)
 
     // Prefer URL query first, then atom, then fallback
+    // Only accept scenarioId from router.query if found in the valid scenarios list.
+    const safeScenarioIdFromQuery =
+        typeof router.query.scenarioId === "string" && scenarioIds.includes(router.query.scenarioId)
+            ? router.query.scenarioId
+            : undefined
     const activeId =
-        (router.query.scenarioId as string | undefined) ?? urlState.scenarioId ?? scenarioIds[0]
+        safeScenarioIdFromQuery ?? (scenarioIds.includes(urlState.scenarioId) ? urlState.scenarioId : undefined) ?? scenarioIds[0]
 
     // Ensure URL/atom always reference a scenario visible in current list
     // Ensure URL/atom correctness
     useEffect(() => {
         if (scenarioIds.length === 0) return
 
+        // Only consider router.query.scenarioId if present in allowed list
+        const safeScenarioIdFromQuery =
+            typeof router.query.scenarioId === "string" && scenarioIds.includes(router.query.scenarioId)
+                ? router.query.scenarioId
+                : undefined
         const currentScenarioId =
-            (router.query.scenarioId as string | undefined) ?? urlState.scenarioId
+            safeScenarioIdFromQuery ?? (scenarioIds.includes(urlState.scenarioId) ? urlState.scenarioId : undefined)
 
-        if (!currentScenarioId || !scenarioIds.includes(currentScenarioId)) {
+        if (!currentScenarioId) {
             // Default to the first scenario for this run when no valid selection/deep-link.
             setUrlState({scenarioId: scenarioIds[0]})
             return
EOF
@@ -32,18 +32,28 @@
const setUrlState = useSetAtom(setUrlStateAtom)

// Prefer URL query first, then atom, then fallback
// Only accept scenarioId from router.query if found in the valid scenarios list.
const safeScenarioIdFromQuery =
typeof router.query.scenarioId === "string" && scenarioIds.includes(router.query.scenarioId)
? router.query.scenarioId
: undefined
const activeId =
(router.query.scenarioId as string | undefined) ?? urlState.scenarioId ?? scenarioIds[0]
safeScenarioIdFromQuery ?? (scenarioIds.includes(urlState.scenarioId) ? urlState.scenarioId : undefined) ?? scenarioIds[0]

// Ensure URL/atom always reference a scenario visible in current list
// Ensure URL/atom correctness
useEffect(() => {
if (scenarioIds.length === 0) return

// Only consider router.query.scenarioId if present in allowed list
const safeScenarioIdFromQuery =
typeof router.query.scenarioId === "string" && scenarioIds.includes(router.query.scenarioId)
? router.query.scenarioId
: undefined
const currentScenarioId =
(router.query.scenarioId as string | undefined) ?? urlState.scenarioId
safeScenarioIdFromQuery ?? (scenarioIds.includes(urlState.scenarioId) ? urlState.scenarioId : undefined)

if (!currentScenarioId || !scenarioIds.includes(currentScenarioId)) {
if (!currentScenarioId) {
// Default to the first scenario for this run when no valid selection/deep-link.
setUrlState({scenarioId: scenarioIds[0]})
return
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +87 to +88
return await axios
.get(`${getAgentaApiUrl()}/human-evaluations/${evaluationId}?project_id=${projectId}`)

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
of this request depends on a
user-provided value
.
The
URL
of this request depends on a
user-provided value
.

Copilot Autofix

AI about 1 month ago

To mitigate this SSRF risk, restrict evaluationId to an expected format before using it in the request path, e.g., ensure it matches allowed characters (such as a MongoDB ObjectId or a UUID). The best way to do this is by validating the evaluationId string in fetchLoadEvaluation before including it in the axios request. If the value fails validation, throw an error or return a controlled response instead of making the HTTP request. The validation should be implemented right at the start of fetchLoadEvaluation to ensure early exit if the input is malicious or malformed.

To implement the change:

  • In web/ee/src/services/human-evaluations/api/index.ts, update the fetchLoadEvaluation function (lines 84–96).
  • Before making the axios request, validate that evaluationId matches the expected format, for example, checking if it's a 24-character hexadecimal string (for Mongo ObjectId), or matches a UUID regex if that's appropriate.
  • If validation fails, log an error and return null (or throw, depending on desired error handling).
  • Import or define a helper validation function if needed.

No code changes are needed in the component/page that reads router.query.evaluation_id.


Suggested changeset 1
web/ee/src/services/human-evaluations/api/index.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/web/ee/src/services/human-evaluations/api/index.ts b/web/ee/src/services/human-evaluations/api/index.ts
--- a/web/ee/src/services/human-evaluations/api/index.ts
+++ b/web/ee/src/services/human-evaluations/api/index.ts
@@ -81,8 +81,16 @@
     return results
 }
 
+// Validate ObjectId: 24-character hex string
+const isValidObjectId = (id: string): boolean => /^[a-fA-F0-9]{24}$/.test(id);
+
 export const fetchLoadEvaluation = async (evaluationId: string) => {
     const {projectId} = getCurrentProject()
+    // SSRF mitigation: Validate evaluationId before interpolating
+    if (!isValidObjectId(evaluationId)) {
+        console.error(`Invalid evaluationId format: ${evaluationId}`)
+        return null
+    }
     try {
         return await axios
             .get(`${getAgentaApiUrl()}/human-evaluations/${evaluationId}?project_id=${projectId}`)
EOF
@@ -81,8 +81,16 @@
return results
}

// Validate ObjectId: 24-character hex string
const isValidObjectId = (id: string): boolean => /^[a-fA-F0-9]{24}$/.test(id);

export const fetchLoadEvaluation = async (evaluationId: string) => {
const {projectId} = getCurrentProject()
// SSRF mitigation: Validate evaluationId before interpolating
if (!isValidObjectId(evaluationId)) {
console.error(`Invalid evaluationId format: ${evaluationId}`)
return null
}
try {
return await axios
.get(`${getAgentaApiUrl()}/human-evaluations/${evaluationId}?project_id=${projectId}`)
Copilot is powered by AI and may make mistakes. Always verify output.
return fromEvaluationResponseToEvaluation(responseData.data)
})
} catch (error) {
console.error(`Error fetching evaluation ${evaluationId}:`, error)

Check failure

Code scanning / CodeQL

Use of externally-controlled format string High

Format string depends on a
user-provided value
.
Format string depends on a
user-provided value
.

Copilot Autofix

AI about 1 month ago

To prevent the risk of user input being interpreted as a format string by console.error, the recommended fix is to use a static string as the format and pass the potentially-tainted value as a separate argument. Specifically, change from:

console.error(`Error fetching evaluation ${evaluationId}:`, error)

to:

console.error("Error fetching evaluation %s:", evaluationId, error)

This ensures that evaluationId will always be treated as a string and not as part of a format string, thus eliminating the opportunity for input-controlled format confusion. Only the specific console.error invocation on line 93 of web/ee/src/services/human-evaluations/api/index.ts needs to be updated. No extra imports or definitions are needed.


Suggested changeset 1
web/ee/src/services/human-evaluations/api/index.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/web/ee/src/services/human-evaluations/api/index.ts b/web/ee/src/services/human-evaluations/api/index.ts
--- a/web/ee/src/services/human-evaluations/api/index.ts
+++ b/web/ee/src/services/human-evaluations/api/index.ts
@@ -90,7 +90,7 @@
                 return fromEvaluationResponseToEvaluation(responseData.data)
             })
     } catch (error) {
-        console.error(`Error fetching evaluation ${evaluationId}:`, error)
+        console.error("Error fetching evaluation %s:", evaluationId, error)
         return null
     }
 }
EOF
@@ -90,7 +90,7 @@
return fromEvaluationResponseToEvaluation(responseData.data)
})
} catch (error) {
console.error(`Error fetching evaluation ${evaluationId}:`, error)
console.error("Error fetching evaluation %s:", evaluationId, error)
return null
}
}
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +115 to +118
return await axios
.get(
`${getAgentaApiUrl()}/human-evaluations/${evaluationTableId}/evaluation_scenarios?project_id=${projectId}`,
)

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
of this request depends on a
user-provided value
.
The
URL
of this request depends on a
user-provided value
.

Copilot Autofix

AI about 1 month ago

The best way to fix this SSRF risk is to validate and sanitize the evaluationTableId before using it in any URL path construction. Since the identifier is expected to represent an evaluation table, we should restrict its format to a known-safe pattern (e.g., MongoDB ObjectId, UUID, or some similar canonical form). In practical terms, this means:

  • In fetchAllLoadEvaluationsScenarios (web/ee/src/services/human-evaluations/api/index.ts), only allow evaluationTableId if it matches a strict regular expression that describes the intended ID format (e.g., 24-character hex string for MongoDB ObjectId, or UUID).
  • Throw/return an error or empty result if the ID is invalid, and optionally log the attempt.
  • Add a helper function in the same file to validate the ID, and use it at the start of the exported method.

You only need to change code in web/ee/src/services/human-evaluations/api/index.ts, specifically around the fetchAllLoadEvaluationsScenarios function. The imported modules are standard/common, so you may add e.g. a regex or a helper validation inline there.


Suggested changeset 1
web/ee/src/services/human-evaluations/api/index.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/web/ee/src/services/human-evaluations/api/index.ts b/web/ee/src/services/human-evaluations/api/index.ts
--- a/web/ee/src/services/human-evaluations/api/index.ts
+++ b/web/ee/src/services/human-evaluations/api/index.ts
@@ -106,10 +106,24 @@
     return response.data
 }
 
+// Helper: only allow 24-character Mongo ObjectId or UUID v4
+function isSafeEvaluationId(id: string): boolean {
+    // Mongo ObjectId: 24 hex chars
+    const mongoId = /^[0-9a-fA-F]{24}$/
+    // UUID v4: xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx
+    const uuidV4 = /^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-4[0-9a-fA-F]{3}-[89abAB][0-9a-fA-F]{3}-[0-9a-fA-F]{12}$/
+    return mongoId.test(id) || uuidV4.test(id)
+}
+
 export const fetchAllLoadEvaluationsScenarios = async (
     evaluationTableId: string,
     evaluation: Evaluation,
 ) => {
+    if (!isSafeEvaluationId(evaluationTableId)) {
+        console.error("Invalid evaluationTableId value supplied:", evaluationTableId)
+        // Optionally: return empty array, null, or throw error
+        return []
+    }
     const {projectId} = getCurrentProject()
 
     return await axios
EOF
@@ -106,10 +106,24 @@
return response.data
}

// Helper: only allow 24-character Mongo ObjectId or UUID v4
function isSafeEvaluationId(id: string): boolean {
// Mongo ObjectId: 24 hex chars
const mongoId = /^[0-9a-fA-F]{24}$/
// UUID v4: xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx
const uuidV4 = /^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-4[0-9a-fA-F]{3}-[89abAB][0-9a-fA-F]{3}-[0-9a-fA-F]{12}$/
return mongoId.test(id) || uuidV4.test(id)
}

export const fetchAllLoadEvaluationsScenarios = async (
evaluationTableId: string,
evaluation: Evaluation,
) => {
if (!isSafeEvaluationId(evaluationTableId)) {
console.error("Invalid evaluationTableId value supplied:", evaluationTableId)
// Optionally: return empty array, null, or throw error
return []
}
const {projectId} = getCurrentProject()

return await axios
Copilot is powered by AI and may make mistakes. Always verify output.
Comment on lines +194 to +197
const response = await axios.put(
`${getAgentaApiUrl()}/human-evaluations/${evaluationTableId}/evaluation_scenario/${evaluationScenarioId}/${evaluationType}?project_id=${projectId}`,
data,
)

Check failure

Code scanning / CodeQL

Server-side request forgery Critical

The
URL
of this request depends on a
user-provided value
.

Copilot Autofix

AI about 1 month ago

To correct the problem, you should validate (sanitize) all user-controlled input used in REST API paths, especially variables coming from the URL query (evaluationScenarioId in this case). The best fix is to ensure evaluationScenarioId matches only the allowed scenario IDs -- which are already present in-memory as part of evaluationScenarios (see usage in both components). This requires, before calling updateEvaluationScenario (or in that method), checking that evaluationScenarioId appears in evaluationScenarios.map(s => s.id). If it is not present, the operation should be aborted or an error raised.

The implementation can be made at the point where scenarioId (from useQueryParam) is passed to updateEvaluationScenarioData (and thus eventually becomes evaluationScenarioId). You should add validation in both web/ee/src/components/Evaluations/EvaluationCardView/index.tsx (where scenarioId is used) and/or in web/ee/src/components/EvaluationTable/SingleModelEvaluationTable.tsx (where id is used for updating scenarios), so that only known IDs from evaluationScenarios are accepted.

You will need to:

  • Add a helper function that checks if a scenarioId is valid.
  • Apply this check before passing scenarioId (or id) to SSRF sinks.
  • Prevent requests with invalid or malicious IDs.

No new imports are strictly necessary.

Suggested changeset 2
web/ee/src/components/EvaluationTable/SingleModelEvaluationTable.tsx
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/web/ee/src/components/EvaluationTable/SingleModelEvaluationTable.tsx b/web/ee/src/components/EvaluationTable/SingleModelEvaluationTable.tsx
--- a/web/ee/src/components/EvaluationTable/SingleModelEvaluationTable.tsx
+++ b/web/ee/src/components/EvaluationTable/SingleModelEvaluationTable.tsx
@@ -188,6 +188,12 @@
         data: Partial<EvaluationScenario>,
         showNotification = true,
     ) => {
+        // SSRF mitigation: Only update scenarios with valid IDs
+        const allowedScenarioIds = evaluationScenarios.map(s => s.id)
+        if (!allowedScenarioIds.includes(id)) {
+            console.warn("Blocked invalid evaluationScenarioId for SSRF protection:", id)
+            return
+        }
         await updateEvaluationScenario(
             evaluation.id,
             id,
EOF
@@ -188,6 +188,12 @@
data: Partial<EvaluationScenario>,
showNotification = true,
) => {
// SSRF mitigation: Only update scenarios with valid IDs
const allowedScenarioIds = evaluationScenarios.map(s => s.id)
if (!allowedScenarioIds.includes(id)) {
console.warn("Blocked invalid evaluationScenarioId for SSRF protection:", id)
return
}
await updateEvaluationScenario(
evaluation.id,
id,
web/ee/src/components/Evaluations/EvaluationCardView/index.tsx
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/web/ee/src/components/Evaluations/EvaluationCardView/index.tsx b/web/ee/src/components/Evaluations/EvaluationCardView/index.tsx
--- a/web/ee/src/components/Evaluations/EvaluationCardView/index.tsx
+++ b/web/ee/src/components/Evaluations/EvaluationCardView/index.tsx
@@ -131,9 +131,16 @@
 
     const depouncedUpdateEvaluationScenario = useCallback(
         debounce((data: Partial<EvaluationScenario>) => {
+            // SSRF mitigation: Only use scenarioId if it is present in evaluationScenarios
+            const allowedScenarioIds = evaluationScenarios.map(s => s.id)
+            if (!allowedScenarioIds.includes(scenarioId)) {
+                // Optionally, log, show an error, or silently return
+                console.warn("Blocked invalid scenarioId for SSRF protection:", scenarioId)
+                return
+            }
             updateEvaluationScenarioData(scenarioId, data)
         }, 800),
-        [scenarioId],
+        [scenarioId, evaluationScenarios],
     )
 
     const onChatChange = (chat: ChatMessage[]) => {
EOF
@@ -131,9 +131,16 @@

const depouncedUpdateEvaluationScenario = useCallback(
debounce((data: Partial<EvaluationScenario>) => {
// SSRF mitigation: Only use scenarioId if it is present in evaluationScenarios
const allowedScenarioIds = evaluationScenarios.map(s => s.id)
if (!allowedScenarioIds.includes(scenarioId)) {
// Optionally, log, show an error, or silently return
console.warn("Blocked invalid scenarioId for SSRF protection:", scenarioId)
return
}
updateEvaluationScenarioData(scenarioId, data)
}, 800),
[scenarioId],
[scenarioId, evaluationScenarios],
)

const onChatChange = (chat: ChatMessage[]) => {
Copilot is powered by AI and may make mistakes. Always verify output.
@junaway junaway changed the title Chore/swap sendgrid by smtp [stale] [chore] Swap sendgrid by smtp Nov 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants