diff --git a/artifacts/views.py b/artifacts/views.py index 9765b9a..0db2e82 100644 --- a/artifacts/views.py +++ b/artifacts/views.py @@ -237,6 +237,9 @@ class ArtifactDetailView(APIView): length_error = validate_field_lengths(data, ["title", "source_name", "source_id"]) if length_error: return length_error + string_error = validate_string_fields(data, ["source_name", "source_id", "source_prompt"]) + if string_error: + return string_error if "title" in data: if not data["title"]: return Response({"detail": "title cannot be blank"}, status=400) @@ -860,6 +863,19 @@ def validate_field_lengths(data, fields): return None +def validate_string_fields(data, fields): + """Rejects non-string JSON types (object/array/number) instead of letting + them reach save(), where Django's CharField/TextField silently coerce via + str() - which for a dict/list produces Python repr() syntax, not JSON, + and is irreversible. + """ + for field in fields: + value = data.get(field) + if value is not None and not isinstance(value, str): + return Response({"detail": f"{field} must be a string"}, status=400) + return None + + def validate_content_shape(kind, content): lower = (content or "").lower() if kind == Artifact.DECK and " 200, response shows "source_name": 12345 (int) + +curl .../api/artifacts/a74de286 # fresh GET, session auth +# -> "source_name": "12345" (string) + +# Actual DB values (confirmed via shell): +# source_name = '12345' +# source_id = "['a', 'b']" <- repr() of the list, not JSON +# source_prompt = "{'nested': 'object'}" <- repr() of the dict, not JSON +``` + +Also reproduced identically on `POST` (create), not just `PATCH` — a +freshly created artifact's immediate response shows +`"source_name": {"a": 1}` (real object) even though what's stored (and +what any later `GET` returns) is the mangled string form. + +## Suggested Fix + +Add a type check for `source_name`, `source_id`, and `source_prompt` +(must be `str` or absent/null) alongside the existing `tags`/`metadata`/ +`tag_colors` checks in `validate_source_fields` (create path) and +`ArtifactDetailView.patch` (update path) — return `400` naming the field, +consistent with #005's approach for `tags`. This also removes the +response/DB inconsistency, since invalid types would never reach `save()` +in the first place. + +## Resolution + +Added `validate_string_fields(data, fields)` in `artifacts/views.py`, +rejecting any non-`None`, non-`str` value with `400`. Called from +`validate_source_fields` (covers `POST`, both text and binary paths) and +directly in `ArtifactDetailView.patch` for `source_name`, `source_id`, +`source_prompt` — matching #005's approach for `tags`. Invalid values never +reach `save()`, so the response/DB inconsistency the issue described can no +longer occur. Verified live against the running dev instance and with new +tests in `tests/api/test_artifacts.py`. diff --git a/tests/api/test_artifacts.py b/tests/api/test_artifacts.py index e889e06..bbd634e 100644 --- a/tests/api/test_artifacts.py +++ b/tests/api/test_artifacts.py @@ -427,3 +427,31 @@ class ArtifactAPITestCase(APITestCase): def test_create_csv_artifact_with_header_only(self): response = self.create_artifact(kind="csv", content="name,score") self.assertEqual(response.status_code, 201) + + # -- source_name/source_id/source_prompt type checks (issue #008) ------- + + def test_create_rejects_non_string_source_name(self): + response = self.create_artifact(source_name=12345) + self.assertEqual(response.status_code, 400) + self.assertIn("source_name", response.data["detail"]) + + def test_create_rejects_non_string_source_id(self): + response = self.create_artifact(source_id=["a", "b"]) + self.assertEqual(response.status_code, 400) + + def test_create_rejects_non_string_source_prompt(self): + response = self.create_artifact(source_prompt={"nested": "object"}) + self.assertEqual(response.status_code, 400) + + def test_patch_rejects_non_string_source_name(self): + artifact_id = self.create_artifact().data["id"] + response = self.client.patch( + reverse("artifact-detail-api", args=[artifact_id]), + {"source_name": 12345}, + format="json", + **self.auth_headers(self.token), + ) + self.assertEqual(response.status_code, 400) + + artifact = Artifact.objects.get(pk=artifact_id) + self.assertEqual(artifact.source_name, "test-agent") # unchanged, not mangled