diff --git a/api/oss/src/core/applications/dtos.py b/api/oss/src/core/applications/dtos.py index 1b186c8ce1..9bdd271c67 100644 --- a/api/oss/src/core/applications/dtos.py +++ b/api/oss/src/core/applications/dtos.py @@ -1,7 +1,7 @@ from typing import Optional, List from uuid import UUID -from pydantic import Field +from pydantic import ConfigDict, Field from oss.src.core.shared.dtos import sync_alias, AliasConfig from oss.src.core.shared.dtos import ( @@ -212,7 +212,7 @@ class ApplicationVariantQuery(WorkflowVariantQuery): class ApplicationRevisionData(WorkflowRevisionData): - pass + model_config = ConfigDict(extra="forbid") class ApplicationRevision( diff --git a/api/oss/src/core/embeds/service.py b/api/oss/src/core/embeds/service.py index 7601802098..8a04fecc53 100644 --- a/api/oss/src/core/embeds/service.py +++ b/api/oss/src/core/embeds/service.py @@ -16,43 +16,6 @@ log = get_module_logger(__name__) -def _normalize_configuration_for_legacy_paths( - configuration: Dict[str, Any], -) -> Dict[str, Any]: - """ - Normalize config shape so embed path extraction works across old/new payloads. - - Some revisions store prompt config under: - - parameters.prompt... - while others use: - - configuration.parameters.prompt... - - The resolver may receive embed paths targeting either shape, so we expose both. - """ - normalized = dict(configuration or {}) - - parameters = normalized.get("parameters") - config_obj = normalized.get("configuration") - - # If only legacy configuration.parameters exists, expose top-level parameters. - if not isinstance(parameters, dict) and isinstance(config_obj, dict): - cfg_params = config_obj.get("parameters") - if isinstance(cfg_params, dict): - normalized["parameters"] = cfg_params - parameters = cfg_params - - # If only top-level parameters exists, expose configuration.parameters too. - if isinstance(parameters, dict): - if not isinstance(config_obj, dict): - config_obj = {} - if not isinstance(config_obj.get("parameters"), dict): - config_obj = dict(config_obj) - config_obj["parameters"] = parameters - normalized["configuration"] = config_obj - - return normalized - - class EmbedsService: """ Centralized service for resolving embedded references. @@ -101,8 +64,6 @@ async def resolve_configuration( Returns: Tuple of (resolved configuration dict, ResolutionInfo metadata) """ - configuration = _normalize_configuration_for_legacy_paths(configuration) - # Create universal resolver with all available services resolver_callback = create_universal_resolver( project_id=project_id, diff --git a/api/oss/src/core/environments/dtos.py b/api/oss/src/core/environments/dtos.py index c24114b22b..5c92065181 100644 --- a/api/oss/src/core/environments/dtos.py +++ b/api/oss/src/core/environments/dtos.py @@ -1,7 +1,7 @@ from typing import Optional, Dict, List from uuid import UUID -from pydantic import BaseModel, Field +from pydantic import BaseModel, ConfigDict, Field from oss.src.core.shared.dtos import ( sync_alias, @@ -128,6 +128,8 @@ class EnvironmentRevisionData(BaseModel): } """ + model_config = ConfigDict(extra="forbid") + references: Optional[Dict[str, Dict[str, Reference]]] = None diff --git a/api/oss/src/core/evaluators/dtos.py b/api/oss/src/core/evaluators/dtos.py index e68f4c53ec..348820f5bf 100644 --- a/api/oss/src/core/evaluators/dtos.py +++ b/api/oss/src/core/evaluators/dtos.py @@ -1,7 +1,7 @@ from typing import Optional, List from uuid import UUID -from pydantic import Field +from pydantic import ConfigDict, Field from oss.src.core.shared.dtos import sync_alias, AliasConfig from oss.src.core.shared.dtos import ( @@ -208,7 +208,7 @@ class EvaluatorVariantQuery(WorkflowVariantQuery): class EvaluatorRevisionData(WorkflowRevisionData): - pass + model_config = ConfigDict(extra="forbid") class EvaluatorRevision( diff --git a/api/oss/src/core/queries/dtos.py b/api/oss/src/core/queries/dtos.py index 3b17bfb9f7..3eedde6001 100644 --- a/api/oss/src/core/queries/dtos.py +++ b/api/oss/src/core/queries/dtos.py @@ -1,7 +1,7 @@ from typing import List, Optional from uuid import UUID -from pydantic import BaseModel, Field +from pydantic import BaseModel, ConfigDict, Field from oss.src.core.tracing.dtos import Filtering, Formatting @@ -139,6 +139,8 @@ class QueryVariantQuery(VariantQuery): class QueryRevisionData(BaseModel): + model_config = ConfigDict(extra="forbid") + formatting: Optional[Formatting] = None filtering: Optional[Filtering] = None windowing: Optional[Windowing] = None diff --git a/api/oss/src/core/testsets/dtos.py b/api/oss/src/core/testsets/dtos.py index 3ab2ffb2f3..744fe93af9 100644 --- a/api/oss/src/core/testsets/dtos.py +++ b/api/oss/src/core/testsets/dtos.py @@ -1,7 +1,7 @@ from typing import Optional, List, Tuple from uuid import UUID -from pydantic import BaseModel, Field +from pydantic import BaseModel, ConfigDict, Field from oss.src.core.shared.dtos import ( sync_alias, @@ -154,6 +154,8 @@ class TestsetVariantQuery(VariantQuery): class TestsetRevisionData(BaseModel): + model_config = ConfigDict(extra="forbid") + testcase_ids: Optional[List[UUID]] = None testcases: Optional[List[Testcase]] = None diff --git a/api/oss/src/core/workflows/dtos.py b/api/oss/src/core/workflows/dtos.py index 7d21391251..c382789809 100644 --- a/api/oss/src/core/workflows/dtos.py +++ b/api/oss/src/core/workflows/dtos.py @@ -54,9 +54,17 @@ WorkflowServiceStreamResponse, # noqa: F401 # JsonSchemas, # noqa: F401 - WorkflowRevisionData, + WorkflowRevisionData as BaseWorkflowRevisionData, ) + +class WorkflowRevisionData(BaseWorkflowRevisionData): + pass + + +WorkflowRevisionCommitData = WorkflowRevisionData + + # aliases ---------------------------------------------------------------------- diff --git a/api/oss/tests/pytest/acceptance/test_revision_commit_extra_forbid.py b/api/oss/tests/pytest/acceptance/test_revision_commit_extra_forbid.py new file mode 100644 index 0000000000..b9c1402871 --- /dev/null +++ b/api/oss/tests/pytest/acceptance/test_revision_commit_extra_forbid.py @@ -0,0 +1,317 @@ +"""Issue #4315 — revision commit endpoints reject unknown top-level `data` keys. + +Every revision-commit endpoint shares the same vulnerability class: an unknown +top-level field inside `data` used to be silently dropped, returning HTTP 200 +with `count: 1` but storing `data: {}`. Setting `extra="forbid"` on each +RevisionData DTO converts that into a 422 with the offending field named. + +These tests guard all six domains uniformly so future scope changes don't +silently regress. +""" + +from uuid import uuid4 + + +# helpers ---------------------------------------------------------------------- + + +def _commit(authed_api, path: str, payload: dict): + """POST and return the response without asserting status.""" + return authed_api("POST", path, json=payload) + + +def _assert_422_names(response, field: str): + assert response.status_code == 422, response.text + assert field in response.text + + +# workflows -------------------------------------------------------------------- + + +def _create_workflow_with_variant(authed_api): + slug = f"wf-{uuid4().hex[:8]}" + response = authed_api( + "POST", + "/workflows/", + json={"workflow": {"slug": slug, "name": slug}}, + ) + assert response.status_code == 200, response.text + workflow = response.json()["workflow"] + + variant_slug = f"{slug}-v" + response = authed_api( + "POST", + "/workflows/variants/", + json={ + "workflow_variant": { + "slug": variant_slug, + "name": variant_slug, + "workflow_id": workflow["id"], + } + }, + ) + assert response.status_code == 200, response.text + return workflow, response.json()["workflow_variant"] + + +def test_commit_workflow_revision_rejects_unknown_data_field(authed_api): + workflow, variant = _create_workflow_with_variant(authed_api) + response = _commit( + authed_api, + "/workflows/revisions/commit", + { + "workflow_revision_commit": { + "slug": uuid4().hex[-12:], + "workflow_id": workflow["id"], + "workflow_variant_id": variant["id"], + "data": {"ag_config": {"prompt": {}}}, + } + }, + ) + _assert_422_names(response, "ag_config") + + +# applications ----------------------------------------------------------------- + + +def _create_application_with_variant(authed_api): + slug = f"app-{uuid4().hex[:8]}" + response = authed_api( + "POST", + "/applications/", + json={ + "application": { + "slug": slug, + "name": slug, + "flags": { + "is_application": True, + "is_evaluator": False, + "is_snippet": False, + }, + } + }, + ) + assert response.status_code == 200, response.text + application = response.json()["application"] + + variant_slug = f"{slug}-v" + response = authed_api( + "POST", + "/applications/variants/", + json={ + "application_variant": { + "slug": variant_slug, + "name": variant_slug, + "flags": { + "is_application": True, + "is_evaluator": False, + "is_snippet": False, + }, + "application_id": application["id"], + } + }, + ) + assert response.status_code == 200, response.text + return application, response.json()["application_variant"] + + +def test_commit_application_revision_rejects_unknown_data_field(authed_api): + application, variant = _create_application_with_variant(authed_api) + response = _commit( + authed_api, + "/applications/revisions/commit", + { + "application_revision_commit": { + "application_id": application["id"], + "application_variant_id": variant["id"], + "data": {"ag_config": {"prompt": {}}}, + } + }, + ) + _assert_422_names(response, "ag_config") + + +# evaluators ------------------------------------------------------------------- + + +def _create_evaluator_with_variant(authed_api): + slug = f"ev-{uuid4().hex[:8]}" + response = authed_api( + "POST", + "/evaluators/", + json={ + "evaluator": { + "slug": slug, + "name": slug, + "flags": { + "is_application": False, + "is_evaluator": True, + "is_snippet": False, + }, + } + }, + ) + assert response.status_code == 200, response.text + evaluator = response.json()["evaluator"] + + variant_slug = f"{slug}-v" + response = authed_api( + "POST", + "/evaluators/variants/", + json={ + "evaluator_variant": { + "slug": variant_slug, + "name": variant_slug, + "flags": { + "is_application": False, + "is_evaluator": True, + "is_snippet": False, + }, + "evaluator_id": evaluator["id"], + } + }, + ) + assert response.status_code == 200, response.text + return evaluator, response.json()["evaluator_variant"] + + +def test_commit_evaluator_revision_rejects_unknown_data_field(authed_api): + evaluator, variant = _create_evaluator_with_variant(authed_api) + response = _commit( + authed_api, + "/evaluators/revisions/commit", + { + "evaluator_revision_commit": { + "evaluator_id": evaluator["id"], + "evaluator_variant_id": variant["id"], + "data": {"ag_config": {"prompt": {}}}, + } + }, + ) + _assert_422_names(response, "ag_config") + + +# testsets --------------------------------------------------------------------- + + +def _create_testset_with_variant(authed_api): + slug = f"ts-{uuid4().hex[:8]}" + response = authed_api( + "POST", + "/testsets/", + json={"testset": {"slug": slug, "name": slug}}, + ) + assert response.status_code == 200, response.text + testset = response.json()["testset"] + + variant_slug = f"{slug}-v" + response = authed_api( + "POST", + "/testsets/variants/", + json={ + "testset_variant": { + "slug": variant_slug, + "name": variant_slug, + "testset_id": testset["id"], + } + }, + ) + assert response.status_code == 200, response.text + return testset, response.json()["testset_variant"] + + +def test_commit_testset_revision_rejects_unknown_data_field(authed_api): + testset, variant = _create_testset_with_variant(authed_api) + response = _commit( + authed_api, + "/testsets/revisions/commit", + { + "testset_revision_commit": { + "slug": uuid4().hex[-12:], + "testset_id": testset["id"], + "testset_variant_id": variant["id"], + "data": {"csvdata": [{"input": "x"}]}, + } + }, + ) + _assert_422_names(response, "csvdata") + + +# queries ---------------------------------------------------------------------- + + +def test_commit_query_revision_rejects_unknown_data_field(authed_api): + slug = uuid4().hex + response = authed_api( + "POST", + "/simple/queries/", + json={ + "query": { + "slug": slug, + "name": f"Test Query {slug}", + "data": {"windowing": {"limit": 50}}, + } + }, + ) + assert response.status_code == 200, response.text + query = response.json()["query"] + + response = _commit( + authed_api, + "/queries/revisions/commit", + { + "query_revision_commit": { + "slug": uuid4().hex[-12:], + "query_id": query["id"], + "query_variant_id": query["variant_id"], + "data": {"surprise": True}, + } + }, + ) + _assert_422_names(response, "surprise") + + +# environments ----------------------------------------------------------------- + + +def _create_environment_with_variant(authed_api): + slug = f"env-{uuid4().hex[:8]}" + response = authed_api( + "POST", + "/environments/", + json={"environment": {"slug": slug, "name": slug}}, + ) + assert response.status_code == 200, response.text + environment = response.json()["environment"] + + variant_slug = f"{slug}-v" + response = authed_api( + "POST", + "/environments/variants/", + json={ + "environment_variant": { + "slug": variant_slug, + "name": variant_slug, + "environment_id": environment["id"], + } + }, + ) + assert response.status_code == 200, response.text + return environment, response.json()["environment_variant"] + + +def test_commit_environment_revision_rejects_unknown_data_field(authed_api): + environment, variant = _create_environment_with_variant(authed_api) + response = _commit( + authed_api, + "/environments/revisions/commit", + { + "environment_revision_commit": { + "slug": uuid4().hex[-12:], + "environment_id": environment["id"], + "environment_variant_id": variant["id"], + "data": {"surprise": {}}, + } + }, + ) + _assert_422_names(response, "surprise") diff --git a/sdks/python/agenta/sdk/models/testsets.py b/sdks/python/agenta/sdk/models/testsets.py index c3e4eb1b64..625707f178 100644 --- a/sdks/python/agenta/sdk/models/testsets.py +++ b/sdks/python/agenta/sdk/models/testsets.py @@ -1,7 +1,7 @@ from typing import List, Optional, Dict, Any from uuid import UUID -from pydantic import BaseModel, Field +from pydantic import BaseModel, ConfigDict, Field from agenta.sdk.models.shared import ( Identifier, @@ -53,6 +53,8 @@ class TestsetFlags(BaseModel): class TestsetRevisionData(BaseModel): + model_config = ConfigDict(extra="forbid") + testcase_ids: Optional[List[UUID]] = None testcases: Optional[List[Testcase]] = None diff --git a/sdks/python/agenta/sdk/models/workflows.py b/sdks/python/agenta/sdk/models/workflows.py index 67dfc3ac21..ebe60f3e5d 100644 --- a/sdks/python/agenta/sdk/models/workflows.py +++ b/sdks/python/agenta/sdk/models/workflows.py @@ -119,6 +119,8 @@ class WorkflowQueryFlags(BaseModel): class WorkflowRevisionData(BaseModel): + model_config = ConfigDict(extra="forbid") + uri: Optional[str] = None url: Optional[str] = None @@ -562,7 +564,7 @@ class EvaluatorVariantIdAlias(AliasConfig): class EvaluatorRevisionData(WorkflowRevisionData): - pass + model_config = ConfigDict(extra="forbid") class EvaluatorFlags(WorkflowFlags): @@ -712,7 +714,7 @@ class ApplicationVariantEdit(WorkflowVariantEdit): class ApplicationRevisionData(WorkflowRevisionData): - pass + model_config = ConfigDict(extra="forbid") class ApplicationRevision( diff --git a/sdks/python/oss/tests/pytest/unit/test_revision_data_extra_forbid.py b/sdks/python/oss/tests/pytest/unit/test_revision_data_extra_forbid.py new file mode 100644 index 0000000000..9f8159b162 --- /dev/null +++ b/sdks/python/oss/tests/pytest/unit/test_revision_data_extra_forbid.py @@ -0,0 +1,36 @@ +"""SDK-side strict validation for *RevisionData DTOs (issue #4315). + +These tests guard the source-of-truth Pydantic config — unknown top-level +fields in `data` payloads must raise instead of being silently dropped. +""" + +import pytest +from pydantic import ValidationError + +from agenta.sdk.models.workflows import WorkflowRevisionData +from agenta.sdk.models.testsets import TestsetRevisionData + + +class TestWorkflowRevisionDataStrict: + def test_known_field_accepted(self): + WorkflowRevisionData(uri="agenta:custom:llm:v0") + + def test_unknown_field_rejected(self): + with pytest.raises(ValidationError) as exc_info: + WorkflowRevisionData(ag_config={"prompt": {"messages": []}}) + assert "ag_config" in str(exc_info.value) + + def test_unknown_field_alongside_known_field_rejected(self): + with pytest.raises(ValidationError) as exc_info: + WorkflowRevisionData(uri="agenta:custom:llm:v0", surprise=True) + assert "surprise" in str(exc_info.value) + + +class TestTestsetRevisionDataStrict: + def test_known_field_accepted(self): + TestsetRevisionData(testcase_ids=[]) + + def test_unknown_field_rejected(self): + with pytest.raises(ValidationError) as exc_info: + TestsetRevisionData(csvdata=[{"input": "x"}]) + assert "csvdata" in str(exc_info.value)