-
Notifications
You must be signed in to change notification settings - Fork 399
[stale] [poc] Add TimeFilter component and integrate time range selection in ObservabilityDashboard and ObservabilityOverview #2911
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
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. |
There was a problem hiding this 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 is a stale proof-of-concept pull request that implements significant refactoring of the evaluations API system, transitioning from a results-based to a steps-based model, removing deprecated functionality, and cleaning up codebase organization.
Key Changes:
- Refactored evaluations API to use "steps" terminology instead of "results"
- Removed deprecated SimpleEvaluations router and legacy applications functionality
- Cleaned up database migration utilities and removed async/await patterns where synchronous is sufficient
- Consolidated imports and restructured entrypoint.py for better organization
Reviewed Changes
Copilot reviewed 111 out of 2320 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| api/oss/src/apis/fastapi/evaluations/router.py | Major refactor replacing "results" with "steps", removing SimpleEvaluationsRouter, updating permissions, and adding archive/unarchive functionality |
| api/oss/src/apis/fastapi/evaluations/models.py | Updated model names from EvaluationResult to EvaluationStep, renamed metrics models for consistency |
| api/oss/src/apis/fastapi/annotations/router.py | Complete rewrite integrating tracing service and evaluators router for annotation management |
| api/oss/src/apis/fastapi/annotations/models.py | Replaced core types with inline definitions, added enums for annotation categorization |
| api/oss/src/init.py | Simplified email blocking logic by removing async PostHog integration |
| api/oss/databases/postgres/migrations/utils.py | Converted async database operations to synchronous |
| api/entrypoint.py | Restructured imports, removed deprecated routers, simplified middleware setup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| span_type_enum = sa.Enum(SpanType, name="tracetype") | ||
| trace_type_enum = sa.Enum(TraceType, name="spantype") |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
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 is named 'tracetype' and trace_type_enum is named 'spantype'. This should be corrected to match the variable names: span_type_enum should use name='spantype' and trace_type_enum should use name='tracetype'.
| 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") |
|
|
||
| # Connect to the new database to grant schema permissions | ||
| new_db_url = f"{DB_PROTOCOL}://{DB_USER}:{DB_PASS}@{DB_HOST}:{DB_PORT}/{new_name}" | ||
| new_db_url = f"postgresql://{DB_USER}:{DB_PASS}@{DB_HOST}:{DB_PORT}/{new_name}" |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded 'postgresql://' protocol differs from the dynamic DB_PROTOCOL variable used elsewhere. For consistency, use DB_PROTOCOL here as well: f\"{DB_PROTOCOL}://{DB_USER}:{DB_PASS}@{DB_HOST}:{DB_PORT}/{new_name}\"
| new_db_url = f"postgresql://{DB_USER}:{DB_PASS}@{DB_HOST}:{DB_PORT}/{new_name}" | |
| new_db_url = f"{DB_PROTOCOL}://{DB_USER}:{DB_PASS}@{DB_HOST}:{DB_PORT}/{new_name}" |
| queue_id_response = EvaluationQueueIdResponse( | ||
| count=1 if _queue_id else 0, | ||
| queue_id=_queue_id, | ||
| queue_id_response = EvaluationQueueResponse( |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong response model used. The function return type is EvaluationQueueIdResponse but EvaluationQueueResponse is being instantiated. This should be EvaluationQueueIdResponse to match the function signature.
| queue_id_response = EvaluationQueueResponse( | |
| queue_id_response = EvaluationQueueIdResponse( |
|
|
||
| from oss.databases.postgres.migrations.core.utils import ( | ||
| check_for_new_migrations as check_for_new_core_migrations, | ||
| check_for_new_migrations as check_for_new_entities_migratons, |
Copilot
AI
Nov 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Corrected spelling of 'migrations' to 'migratons' in the alias.
| const runRes = await axios.get( | ||
| `/preview/evaluations/runs/${evaluationTableId}?project_id=${projectId}`, | ||
| ) |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
URL
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 14 days ago
To fix this SSRF risk, the code should strictly validate that evaluationTableId is a safe value—either matching a known pattern (e.g. a UUID) or selected from a list of allowed IDs. For this scenario, an effective and minimal mitigation is to ensure evaluationTableId is a string matching the expected format of an evaluation ID (for example, a UUID v4, if that's typical), rejecting any values that don't meet the pattern. This should be implemented in the hook useEvaluationRunData: before using evaluationTableId, validate it and handle invalid IDs appropriately (e.g., skip the request and return null, or log and display an error).
The specific fix is to check that evaluationTableId matches a regex pattern for UUIDs or positive integers (adjust pattern based on what IDs are expected). This validation can be performed just before firing off the axios request in fetchAndEnrichPreviewRun. If invalid, skip the request and do not interact with the backend.
No external libraries are required for simple regex matching.
-
Copy modified lines R78-R95
| @@ -75,6 +75,24 @@ | ||
|
|
||
| // New fetcher for preview runs that fetches and enriches with testsetData | ||
| const fetchAndEnrichPreviewRun = useCallback(async () => { | ||
| // Validate that evaluationTableId is a UUID (or other valid format) | ||
| // UUID v4 regex: /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i | ||
| if ( | ||
| !evaluationTableId || | ||
| typeof evaluationTableId !== "string" || | ||
| !/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i.test(evaluationTableId) | ||
| ) { | ||
| // Optionally log the bad input | ||
| 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 | ||
| } | ||
| evalAtomStore().set(loadingStateAtom, (draft) => { | ||
| draft.isLoadingEvaluation = true | ||
| draft.activeStep = "eval-run" |
| 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
URL
user-provided value
The
URL
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 14 days ago
To fix this SSRF issue, the code must restrict and validate user input before using it in the URL path of a server-side HTTP request. The evaluation ID, received from the query string, should be allowed only if it matches safe, expected formats – e.g., a fixed-length alphanumeric string, UUID, or an ID found in a known allowlist. Client-side validation can supplement backend protections, reducing risk. The best way is to validate all IDs passed into fetchEvaluation, fetchAllEvaluationScenarios, etc. (in web/ee/src/services/evaluations/api/index.ts) to allow only safe values.
Concretely:
- Implement a validation function (e.g.,
isSafeEvaluationId) that checks each evaluationId for type, length, and allowed characters (e.g., UUID v4 format, or simple regex for alphanumeric IDs). - Use this function to filter the array passed to
fetchAllComparisonResultsand propagate through all API calls that use evaluationIds. - In particular, in
fetchAllComparisonResults(and helper functions likefetchEvaluation,fetchAllEvaluationScenarios), reject invalid IDs with an error or by filtering. - Add this validation function in
web/ee/src/services/evaluations/api/index.ts. - Fix the call in
EvaluationCompareMode(inweb/ee/src/components/pages/evaluations/evaluationCompare/EvaluationCompare.tsx) to pass only validated IDs to the service function.
This approach preserves original behavior for valid IDs, eliminates attack paths for SSRF, and clarifies acceptable input boundaries.
-
Copy modified line R29 -
Copy modified lines R31-R36 -
Copy modified line R153 -
Copy modified line R206 -
Copy modified lines R235-R238
| @@ -26,8 +26,14 @@ | ||
| import similarityImg from "@/oss/media/transparency.png" | ||
| import {fetchTestset} from "@/oss/services/testsets/api" | ||
|
|
||
| //Prefix convention: | ||
| // Prefix convention: | ||
| // - fetch: GET single entity from server | ||
| // SSRF protection: restrict evaluation IDs to UUID v4 or strict alphanumeric (adjust as needed) | ||
| const UUID_V4_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; | ||
| export function isSafeEvaluationId(id: string) { | ||
| // Accept UUIDv4 only; or adjust for other safe formats if needed | ||
| return typeof id === "string" && UUID_V4_REGEX.test(id.trim()); | ||
| } | ||
| // - fetchAll: GET all entities from server | ||
| // - create: POST data to server | ||
| // - update: PUT data to server | ||
| @@ -145,6 +150,7 @@ | ||
| } | ||
|
|
||
| export const fetchEvaluation = async (evaluationId: string) => { | ||
| if (!isSafeEvaluationId(evaluationId)) throw new Error("Invalid evaluationId"); | ||
| const {projectId} = getCurrentProject() | ||
|
|
||
| const response = await axios.get(`/evaluations/${evaluationId}?project_id=${projectId}`) | ||
| @@ -197,6 +203,7 @@ | ||
|
|
||
| // Evaluation Scenarios | ||
| export const fetchAllEvaluationScenarios = async (evaluationId: string) => { | ||
| if (!isSafeEvaluationId(evaluationId)) throw new Error("Invalid evaluationId"); | ||
| const {projectId} = getCurrentProject() | ||
|
|
||
| const [{data: evaluationScenarios}, evaluation] = await Promise.all([ | ||
| @@ -225,7 +232,10 @@ | ||
|
|
||
| // Comparison | ||
| export const fetchAllComparisonResults = async (evaluationIds: string[]) => { | ||
| const scenarioGroups = await Promise.all(evaluationIds.map(fetchAllEvaluationScenarios)) | ||
| // SSRF protection: filter unsafe user-supplied IDs | ||
| const safeIds = evaluationIds.filter(isSafeEvaluationId); | ||
| if (safeIds.length === 0) throw new Error("No valid evaluation IDs provided."); | ||
| const scenarioGroups = await Promise.all(safeIds.map(fetchAllEvaluationScenarios)); | ||
| const testset: TestSet = await fetchTestset(scenarioGroups[0][0].evaluation?.testset?.id) | ||
|
|
||
| const inputsNameSet = new Set<string>() |
-
Copy modified line R26 -
Copy modified lines R79-R82
| @@ -23,7 +23,7 @@ | ||
| import {getStringOrJson} from "@/oss/lib/helpers/utils" | ||
| import {variantNameWithRev} from "@/oss/lib/helpers/variantHelper" | ||
| import {ComparisonResultRow, EvaluatorConfig, JSSTheme, TestSet, _Evaluation} from "@/oss/lib/Types" | ||
| import {fetchAllComparisonResults} from "@/oss/services/evaluations/api" | ||
| import {fetchAllComparisonResults, isSafeEvaluationId} from "@/oss/services/evaluations/api" | ||
|
|
||
| import {LongTextCellRenderer} from "../cellRenderers/cellRenderers" | ||
| import EvaluationErrorModal from "../EvaluationErrorProps/EvaluationErrorModal" | ||
| @@ -76,7 +76,10 @@ | ||
| const classes = useStyles() | ||
| const {appTheme} = useAppTheme() | ||
| const [evaluationIdsStr = ""] = useQueryParam("evaluations") | ||
| const evaluationIdsArray = evaluationIdsStr.split(",").filter((item) => !!item) | ||
| // SSRF mitigation: filter unsafe IDs before using further | ||
| const evaluationIdsArray = evaluationIdsStr | ||
| .split(",") | ||
| .filter((item) => !!item && isSafeEvaluationId(item)); | ||
| const [evalIds, setEvalIds] = useState(evaluationIdsArray) | ||
| const [hiddenVariants, setHiddenVariants] = useState<string[]>([]) | ||
| const [fetching, setFetching] = useState(false) |
| 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
URL
user-provided value
The
URL
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 14 days ago
To fix the vulnerability, restrict the user-controlled evaluationId before it is interpolated into any path, ensuring only expected values (such as UUID strings) are allowed. In fetchAllEvaluationScenarios, validate that evaluationId matches the required format (likely a UUID). If the ID does not match, throw an error or ignore it. This can be done via a regular expression check inside fetchAllEvaluationScenarios. All locations where you iterate over evaluationIds (from query string) must ensure this validation as well, so it is best to centralize it in one place.
File/region to change:
web/ee/src/services/evaluations/api/index.ts: Before usingevaluationIdin the outgoing request insidefetchAllEvaluationScenarios.- Add a helper function in the same file to check if a string is a valid UUID (or the correct ID format).
What is needed:
- Add a UUID validation helper (either custom regex or by using a well-known utility).
- Use the helper in
fetchAllEvaluationScenariosto ensure only valid IDs are used — throw an Error (or return empty) if invalid. - Optionally, filter the input array to only process valid IDs in
fetchAllComparisonResults.
-
Copy modified lines R198-R202 -
Copy modified lines R205-R207 -
Copy modified lines R234-R238
| @@ -195,8 +195,16 @@ | ||
| }) | ||
| } | ||
|
|
||
| // Helper to validate evaluationId is a UUID (v4) | ||
| const UUID_V4_REGEX = /^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i; | ||
| function isValidUUID(str: string): boolean { | ||
| return UUID_V4_REGEX.test(str); | ||
| } | ||
| // Evaluation Scenarios | ||
| export const fetchAllEvaluationScenarios = async (evaluationId: string) => { | ||
| if (!isValidUUID(evaluationId)) { | ||
| throw new Error("Invalid evaluationId format."); | ||
| } | ||
| const {projectId} = getCurrentProject() | ||
|
|
||
| const [{data: evaluationScenarios}, evaluation] = await Promise.all([ | ||
| @@ -225,7 +231,11 @@ | ||
|
|
||
| // Comparison | ||
| export const fetchAllComparisonResults = async (evaluationIds: string[]) => { | ||
| const scenarioGroups = await Promise.all(evaluationIds.map(fetchAllEvaluationScenarios)) | ||
| const validIds = evaluationIds.filter(isValidUUID) | ||
| if (!validIds.length) { | ||
| throw new Error("No valid evaluation IDs supplied."); | ||
| } | ||
| const scenarioGroups = await Promise.all(validIds.map(fetchAllEvaluationScenarios)) | ||
| const testset: TestSet = await fetchTestset(scenarioGroups[0][0].evaluation?.testset?.id) | ||
|
|
||
| const inputsNameSet = new Set<string>() |
| 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
URL
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 14 days ago
To fix this SSRF vulnerability, we should strictly validate the scenarioId before using it in a URL for an outgoing request. The best way is to ensure that scenarioId conforms to the expected format (almost certainly a UUID v4—since the codebase uses UUIDs generically) before including it in the request path. We can use a regular expression to verify the UUID format: /^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$/. If the value fails validation, we should throw an error, or at minimum, skip making the external request.
Specifically:
- In file
web/ee/src/services/evaluations/workerUtils.ts, insideupdateScenarioStatusRemote, ensurescenarioIdis a valid UUID before using it in fetch URLs. - The easiest way is to add a helper function in that file, above or inside the method, to check that
scenarioIdmeets the expected criteria. - If not valid, log (in dev) and return early—do not send fetch requests with tainted data.
-
Copy modified lines R9-R13 -
Copy modified lines R21-R27
| @@ -6,6 +6,11 @@ | ||
| /** | ||
| * Update scenario status from a WebWorker / non-axios context. | ||
| */ | ||
| // Helper to validate UUID v4 format | ||
| function isValidUuidV4(id: string): boolean { | ||
| return /^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}$/.test(id); | ||
| } | ||
|
|
||
| export async function updateScenarioStatusRemote( | ||
| apiUrl: string, | ||
| jwt: string, | ||
| @@ -13,6 +18,13 @@ | ||
| status: EvaluationStatus, | ||
| projectId: string, | ||
| ): Promise<void> { | ||
| // SSRF mitigation: Validate scenarioId as UUID v4 before using in URL | ||
| if (!isValidUuidV4(scenarioId)) { | ||
| if (process.env.NODE_ENV !== "production") { | ||
| console.warn(`Potential SSRF attempt: invalid scenarioId "${scenarioId}"`); | ||
| } | ||
| return; | ||
| } | ||
| try { | ||
| // 1. fetch full scenario (backend requires full object on PATCH) | ||
| const res = await fetch( |
| return await axios | ||
| .get(`${getAgentaApiUrl()}/human-evaluations/${evaluationId}?project_id=${projectId}`) |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
URL
user-provided value
The
URL
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 14 days ago
The best way to fix this problem is to validate and sanitize evaluationId before using it to construct outgoing requests. Since it's expected to represent a resource ID, restrict its format to a known-safe pattern (e.g., Mongo ObjectId, UUID). If it fails validation, prevent the axios request or gracefully handle the error.
Specifically, in fetchLoadEvaluation(evaluationId: string), add a validation step before interpolating it into the URL. If evaluationId is not a valid ID, return null or throw an error before reaching axios.
You'll need to:
- Add a helper to validate the
evaluationIdformat (for MongoDB ObjectId: length 24, hex string; for UUID: RFC4122 format; choose as appropriate). - In
fetchLoadEvaluation, check format before making the axios request. - Optionally, inform upstream consumers (in hooks/components) if provided ID is invalid.
-
Copy modified lines R84-R88 -
Copy modified lines R91-R94
| @@ -81,8 +81,17 @@ | ||
| return results | ||
| } | ||
|
|
||
| // Helper: check MongoDB ObjectId (24 hex chars) | ||
| function isValidObjectId(id: string): boolean { | ||
| return /^[a-f\d]{24}$/i.test(id); | ||
| } | ||
|
|
||
| export const fetchLoadEvaluation = async (evaluationId: string) => { | ||
| const {projectId} = getCurrentProject() | ||
| if (!isValidObjectId(evaluationId)) { | ||
| console.error(`Invalid evaluationId: ${evaluationId}`); | ||
| return null; | ||
| } | ||
| try { | ||
| return await axios | ||
| .get(`${getAgentaApiUrl()}/human-evaluations/${evaluationId}?project_id=${projectId}`) |
| 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
user-provided value
Format string depends on a
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 14 days ago
To fix the "externally-controlled format string" issue, we should avoid embedding externally-controlled values (like evaluationId from query parameters) within a format string that is passed along with other arguments to console.error. Instead, the untrusted input should be passed as an argument corresponding to a safe string placeholder ("%s") or, more simply, the first string argument should contain any format specifiers explicitly, and all untrusted values should be provided as separate arguments.
In this code (web/ee/src/services/human-evaluations/api/index.ts), change
console.error(`Error fetching evaluation ${evaluationId}:`, error)to
console.error("Error fetching evaluation %s:", evaluationId, error)This way, the format string is fixed, and evaluationId is injected as a value, avoiding any unexpected result from user-controlled format specifiers.
No new methods or special imports are needed; this is purely a string formatting change.
-
Copy modified line R93
| @@ -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 | ||
| } | ||
| } |
| return await axios | ||
| .get( | ||
| `${getAgentaApiUrl()}/human-evaluations/${evaluationTableId}/evaluation_scenarios?project_id=${projectId}`, | ||
| ) |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
URL
user-provided value
The
URL
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 14 days ago
The main goal is to restrict evaluationTableId to a safe, expected format (e.g., a UUID, a database string ID) before incorporating it into the request path. To do this:
- Sanitize the
evaluationTableIdparameter infetchAllLoadEvaluationsScenarios(). Ideally, restrict it with a regex to safe characters (commonly alphanumeric, dash, underscore). - If the project expects DB ids to be UUIDs (or similar patterns), only accept matching patterns, otherwise restrict to a conservative charset.
- If invalid, raise or handle the error gracefully (e.g., throw or return an empty result).
- Consider applying the same restriction at the top of the chain (in the hook, or as close to the origin as possible), but since we can only change shown snippets, we do it in
fetchAllLoadEvaluationsScenariositself. - No new methods or external packages are strictly required for basic validation.
-
Copy modified lines R113-R116
| @@ -110,6 +110,10 @@ | ||
| evaluationTableId: string, | ||
| evaluation: Evaluation, | ||
| ) => { | ||
| // Only allow alphanumeric, dash, and underscore to prevent injection (adjust pattern as necessary) | ||
| if (!/^[a-zA-Z0-9_-]+$/.test(evaluationTableId)) { | ||
| throw new Error("Invalid evaluationTableId"); | ||
| } | ||
| const {projectId} = getCurrentProject() | ||
|
|
||
| return await axios |
| 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
URL
user-provided value
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 14 days ago
To fix this issue, we need to ensure that evaluationScenarioId (and ideally also evaluationTableId) are validated before they are used as part of a URL in the outgoing HTTP request. The safest approach is to restrict these IDs to a safe set of characters, such as only alphanumeric (and hyphens/underscores if required), matching the format such IDs are expected to have in the application (ideally, they should be UUIDs or database-generated IDs). This prevents attackers from injecting special characters, slashes, or sequences such as "../" into the URL, which could allow SSRF or path traversal.
The fix will be implemented in web/ee/src/services/human-evaluations/api/index.ts within the updateEvaluationScenario function. We will add a simple validation (e.g., a regex test) for evaluationScenarioId and evaluationTableId at the beginning of the function. If validation fails, the function will throw an error, thereby blocking the request and preventing any risky URL construction.
No external dependencies are required; we can use JavaScript's built-in regex support.
-
Copy modified lines R192-R199
| @@ -189,6 +189,14 @@ | ||
| data: GenericObject, | ||
| evaluationType: EvaluationType, | ||
| ) => { | ||
| // Validate IDs: only allow UUID or database ID (alphanumeric plus dash/underscore) | ||
| const idPattern = /^[a-zA-Z0-9-_]+$/; | ||
| if (!idPattern.test(evaluationTableId)) { | ||
| throw new Error("Invalid evaluationTableId"); | ||
| } | ||
| if (!idPattern.test(evaluationScenarioId)) { | ||
| throw new Error("Invalid evaluationScenarioId"); | ||
| } | ||
| const {projectId} = getCurrentProject() | ||
|
|
||
| const response = await axios.put( |
[STALE][POC] feat(observability): add TimeFilter component and integrate time range selection in ObservabilityDashboard and ObservabilityOverview