Fix 10 issues found in high-effort review of main

CHANGELOG.md

@@ -2,6 +2,31 @@
 
 All notable changes to FastAdmin are documented in this file.
 
+## Unreleased
+
+Bug-fix follow-up to the 0.8.1 audit. No public API changes.
+
+- **SQLAlchemy**: relationship (m2m) filter values are coerced to `int` for
+  integer primary keys (previously raised HTTP 500 on strictly typed drivers
+  such as PostgreSQL/asyncpg), and unsupported lookups on relation fields
+  (e.g. `icontains`) are rejected with HTTP 422 instead of silently matching
+  by primary-key equality.
+- **Pony ORM**: the foreign-key filter rewrite no longer corrupts filters on
+  plain columns whose name merely ends with `_id`, and list-view search now
+  runs as a single OR'd WHERE clause instead of materializing every matching
+  row once per search field.
+- **Flask**: `HTTPException`s with an attached response (`abort(Response(...))`)
+  are passed through unchanged, error headers (`Allow`, `WWW-Authenticate`,
+  `Retry-After`, ...) are preserved on JSON error responses, and unhandled
+  errors are logged with their traceback.
+- **Filtering**: `IS NULL` is expressible again on text columns via
+  `field__exact=null` (substring lookups keep the literal string `"null"`).
+- **Frontend**: stop corrupting text fields whose content is the literal word
+  `"true"`/`"false"` on the change form (booleans are now only coerced for
+  boolean widgets), stop shape-based date detection for fields known to have
+  no date widget, and pass the model configuration directly to
+  `transformDataFromServer` so every call site gets the safe behavior.
+
 ## 0.8.1
 
 Bug-fix release from a full-source audit of `main`. No public API changes.

fastadmin/api/frameworks/flask/app.py

@@ -33,11 +33,19 @@ def default(self, o):
 @app.errorhandler(Exception)
 def exception_handler(exc):
     if isinstance(exc, HTTPException):
+        if exc.response is not None:
+            # abort(Response(...)) attached a fully-formed response; pass it
+            # through untouched instead of replacing it with a generic error.
+            return exc
         # Return API errors as JSON {"detail": ...} with the proper HTTP status
         # so the shared React frontend can read the message (matching the Django
         # and FastAPI integrations), instead of werkzeug's default HTML page.
-        return {"detail": exc.description}, exc.code or 500
-    # Unhandled server error: log it server-side but never leak internals to the
-    # client, and return a real HTTP 500 (a bare dict would be sent as HTTP 200).
-    logger.error("Unhandled admin error: %s", exc)
+        # Keep the exception's headers (Allow, WWW-Authenticate, Retry-After, ...)
+        # apart from its text/html Content-Type, which the JSON body replaces.
+        headers = [(k, v) for k, v in exc.get_headers() if k.lower() != "content-type"]
+        return {"detail": exc.description}, exc.code or 500, headers
+    # Unhandled server error: log it (with traceback — this is the only place it
+    # is captured) but never leak internals to the client, and return a real
+    # HTTP 500 (a bare dict would be sent as HTTP 200).
+    logger.error("Unhandled admin error: %s", exc, exc_info=exc)
     return {"detail": "Internal server error."}, 500

fastadmin/api/helpers.py

@@ -24,17 +24,26 @@
 def sanitize_filter_value(
     value: str | list,
     field: ModelFieldWidgetSchema | None = None,
+    condition: str = "exact",
 ) -> bool | None | str | list:
     """Sanitize value (string or list for __in filters).
 
     :params value: a value (str or list of str for __in).
     :params field: the field being filtered, used to decide whether the
         "true"/"false"/"null" literals should be coerced (skipped for text fields).
+    :params condition: the filter lookup (exact/in/icontains/...), used to keep
+        IS NULL expressible on text fields.
     :return: A sanitized value.
     """
     if isinstance(value, list):
-        return [sanitize_filter_value(v, field) for v in value]
+        return [sanitize_filter_value(v, field, condition) for v in value]
     if field is not None and field.filter_widget_type in TEXT_FILTER_WIDGET_TYPES:
+        # Text content may legitimately be the words "true"/"false"/"null", so
+        # they are kept literal — except "null" on an exact lookup, which stays
+        # the only way to express IS NULL on a text column (match the literal
+        # word via contains/icontains instead).
+        if value == "null" and condition == "exact":
+            return None
         return value
     match value:
         case "false":
@@ -109,7 +118,8 @@ def build_query_filters(
             continue
         field_name = key.partition("__")[0]
         field = next((f for f in fields if f.name == field_name), None)
-        result[sanitize_filter_key(key, fields)] = sanitize_filter_value(value, field)
+        sanitized_key = sanitize_filter_key(key, fields)
+        result[sanitized_key] = sanitize_filter_value(value, field, sanitized_key[1])
     return result
 
 

fastadmin/api/service.py

@@ -103,10 +103,13 @@ def _validate_filters(admin_model: Any, filters: dict, exclude_filter_fields: tu
 
         The filterable field set is ``list_filter`` when the admin defines it,
         otherwise the serialized field set; the lookup suffix must be one of
-        ALLOWED_FILTER_CONDITIONS.
+        ALLOWED_FILTER_CONDITIONS. Relation (m2m) fields only support pk
+        equality/membership, so other lookups are rejected here instead of
+        failing (or silently degrading) inside the ORM adapters.
         """
         list_filter = getattr(admin_model, "list_filter", ())
         allowlist = set(list_filter) if list_filter else fields
+        m2m_fields = {f.name for f in admin_model.get_model_fields_with_widget_types() if f.is_m2m}
         for k in filters:
             if k in exclude_filter_fields:
                 continue
@@ -115,6 +118,8 @@ def _validate_filters(admin_model: Any, filters: dict, exclude_filter_fields: tu
                 raise AdminApiException(422, detail=f"Filter by {k} is not allowed")
             if field not in allowlist:
                 raise AdminApiException(422, detail=f"Filter by {k} is not allowed")
+            if field in m2m_fields and condition and condition not in ("exact", "in"):
+                raise AdminApiException(422, detail=f"Filter by {k} is not allowed")
 
     @staticmethod
     def _bind_admin_context(

fastadmin/models/orms/ponyorm.py

@@ -259,8 +259,16 @@ def orm_get_list(
                 field = field_with_condition[0]
                 condition = field_with_condition[1]
                 model_pk_name = self.get_model_pk_name(self.model_cls)
-                if field.endswith(f"_{model_pk_name}"):
-                    field = field.replace(f"_{model_pk_name}", f".{model_pk_name}")
+                # A "<relation>_<pk>" key (added by sanitize_filter_key for fk
+                # fields) is rewritten to the "<relation>.<pk>" path — but only
+                # when the base attribute really is a relation, so a plain
+                # column that merely ends with "_id" is left untouched.
+                pk_suffix = f"_{model_pk_name}"
+                if field.endswith(pk_suffix):
+                    base_name = field[: -len(pk_suffix)]
+                    base_attr = getattr(self.model_cls, base_name, None) if base_name else None
+                    if base_attr is not None and getattr(base_attr, "is_relation", False):
+                        field = f"{base_name}.{model_pk_name}"
                 # Bind the user-supplied value as a local (`fval`) so Pony
                 # resolves it as a query parameter. Only the field path — built
                 # from the admin's validated/allowlisted field names — is
@@ -272,21 +280,16 @@ def orm_get_list(
 
         search_fields = list(self.search_fields)
         if search and search_fields:
-            model_pk_name = self.get_model_pk_name(self.model_cls)
-            ids = []
             # Bind the user-supplied search term as a local so Pony resolves it
-            # as a parameter. Only the field path (from the admin's trusted
-            # search_fields config) is interpolated — never the search value.
+            # as a parameter. Only the field paths (from the admin's trusted
+            # search_fields config) are interpolated — never the search value.
+            # All fields are OR-ed into a single filter so the search runs as
+            # one WHERE clause instead of materializing matching rows per field.
             search_term = search.lower()  # noqa: F841 (referenced by Pony filter string)
-            for search_field in search_fields:
-                pony_search_field = search_field.replace("__", ".")
-                qs_ids = qs.filter(f"search_term in m.{pony_search_field}.lower()")
-                objs = list(qs_ids)
-                ids += [getattr(o, model_pk_name) for o in objs]
-            # Bind the collected pks as a local so Pony resolves it as a
-            # parameter; only the trusted pk field path is interpolated.
-            pk_ids = set(ids)  # noqa: F841 (referenced by Pony filter string)
-            qs = qs.filter(f"m.{model_pk_name} in pk_ids")
+            search_expr = " or ".join(
+                f"search_term in m.{search_field.replace('__', '.')}.lower()" for search_field in search_fields
+            )
+            qs = qs.filter(search_expr)
 
         ordering = [sort_by] if sort_by else self.ordering
         if ordering:

fastadmin/models/orms/sqlalchemy.py

@@ -340,9 +340,17 @@ def order_column(ordering_field: str):
                     if related_mapper is not None:
                         # Relationship field (e.g. m2m): SQLAlchemy rejects a
                         # scalar comparison against a collection, so match on the
-                        # related row's pk via any()/has() instead.
+                        # related row's pk via any()/has() instead. Only pk
+                        # equality/membership is meaningful here — anything else
+                        # must fail loudly rather than degrade to an equality
+                        # match (the service layer rejects these with a 422).
+                        if condition not in ("exact", "in"):
+                            raise ValueError(f"Filter condition {condition!r} is not supported for relation {field!r}")
                         related_cls = related_mapper.class_
                         related_pk = getattr(related_cls, self.get_model_pk_name(related_cls))
+                        if isinstance(related_pk.expression.type, BIGINT | Integer):
+                            with contextlib.suppress(ValueError, TypeError):
+                                value = [int(x) for x in value] if condition == "in" else int(value)
                         match_expr = related_pk.in_(value) if condition == "in" else related_pk == value
                         if getattr(rel_property, "uselist", True):
                             q.append(model_field.any(match_expr))

frontend/src/components/async-select/index.tsx

@@ -28,10 +28,7 @@ import { getFetcher, patchFetcher, postFetcher } from "@/fetchers/fetchers";
 import { getConfigurationModel } from "@/helpers/configuration";
 import { handleError } from "@/helpers/forms";
 import { getTitleFromModel } from "@/helpers/title";
-import {
-  getChangeWidgetTypes,
-  transformDataFromServer,
-} from "@/helpers/transform";
+import { transformDataFromServer } from "@/helpers/transform";
 import { EModelPermission } from "@/interfaces/configuration";
 import { ConfigurationContext } from "@/providers/ConfigurationProvider";
 
@@ -90,10 +87,7 @@ export const AsyncSelect: React.FC<IAsyncSelect> = ({
   const asyncSelectChangeInitialValues = useMemo(
     () =>
       initialChangeValues != null
-        ? transformDataFromServer(
-            initialChangeValues,
-            getChangeWidgetTypes(modelConfiguration),
-          )
+        ? transformDataFromServer(initialChangeValues, modelConfiguration)
         : undefined,
     [initialChangeValues, modelConfiguration],
   );

frontend/src/components/inline-widget/index.tsx

@@ -30,7 +30,6 @@ import {
 import { handleError } from "@/helpers/forms";
 import { getTitleFromModel } from "@/helpers/title";
 import {
-  getChangeWidgetTypes,
   transformDataFromServer,
   transformDataToServer,
   transformFiltersToServer,
@@ -152,10 +151,7 @@ export const InlineWidget: React.FC<IInlineWidget> = ({
   const inlineChangeInitialValues = useMemo(
     () =>
       initialChangeValues != null
-        ? transformDataFromServer(
-            initialChangeValues,
-            getChangeWidgetTypes(modelConfiguration),
-          )
+        ? transformDataFromServer(initialChangeValues, modelConfiguration)
         : undefined,
     [initialChangeValues, modelConfiguration],
   );

frontend/src/containers/change/index.tsx

@@ -28,7 +28,6 @@ import { getConfigurationModel } from "@/helpers/configuration";
 import { handleError } from "@/helpers/forms";
 import { getTitleFromModel } from "@/helpers/title";
 import {
-  getChangeWidgetTypes,
   transformDataFromServer,
   transformDataToServer,
 } from "@/helpers/transform";
@@ -62,10 +61,7 @@ export const Change: React.FC = () => {
   const initialValues = useMemo(
     () =>
       initialChangeValues != null
-        ? transformDataFromServer(
-            initialChangeValues,
-            getChangeWidgetTypes(modelConfiguration),
-          )
+        ? transformDataFromServer(initialChangeValues, modelConfiguration)
         : undefined,
     [initialChangeValues, modelConfiguration],
   );

frontend/src/helpers/transform.test.tsx

@@ -232,21 +232,71 @@ describe("transform", () => {
         "12:00:00",
       );
     });
+    it("keeps a boolean-looking string as-is for a non-boolean widget", () => {
+      expect(transformValueFromServer("false", EFieldWidgetType.Input)).toBe(
+        "false",
+      );
+      expect(transformValueFromServer("True", EFieldWidgetType.TextArea)).toBe(
+        "True",
+      );
+    });
+    it("coerces boolean strings for boolean widgets", () => {
+      expect(transformValueFromServer("false", EFieldWidgetType.Switch)).toBe(
+        false,
+      );
+      expect(transformValueFromServer("true", EFieldWidgetType.Checkbox)).toBe(
+        true,
+      );
+    });
+    it("keeps real booleans regardless of widget type", () => {
+      expect(transformValueFromServer(true, EFieldWidgetType.Input)).toBe(true);
+      expect(transformValueFromServer(false, null)).toBe(false);
+    });
+    it("skips shape detection when the widget is known to be absent", () => {
+      expect(transformValueFromServer("2024-01-15", null)).toBe("2024-01-15");
+      expect(transformValueFromServer("false", null)).toBe("false");
+    });
   });
 
   describe("transformDataFromServer", () => {
     it("transforms all values", () => {
       const r = transformDataFromServer({ d: "2024-01-15" });
       expect(r).toHaveProperty("d");
     });
-    it("respects per-field widget types", () => {
+    it("respects per-field widget types from the model configuration", () => {
       const r = transformDataFromServer(
         { at: "2024-01-15", code: "2024-01-15" },
-        { at: EFieldWidgetType.DatePicker, code: EFieldWidgetType.Input },
+        {
+          fields: [
+            {
+              name: "at",
+              change_configuration: {
+                form_widget_type: EFieldWidgetType.DatePicker,
+              },
+            },
+            {
+              name: "code",
+              change_configuration: {
+                form_widget_type: EFieldWidgetType.Input,
+              },
+            },
+          ],
+        } as any,
       );
       expect(dayjs.isDayjs(r.at)).toBe(true);
       expect(r.code).toBe("2024-01-15");
     });
+    it("does not shape-detect fields without a change widget when a configuration is given", () => {
+      const r = transformDataFromServer(
+        { code: "2024-01-15", flag: "false", unknown: "2024-01-15" },
+        {
+          fields: [{ name: "code", change_configuration: {} }],
+        } as any,
+      );
+      expect(r.code).toBe("2024-01-15");
+      expect(r.flag).toBe("false");
+      expect(r.unknown).toBe("2024-01-15");
+    });
   });
 
   describe("getChangeWidgetTypes", () => {
@@ -262,7 +312,7 @@ describe("transform", () => {
           { name: "code", change_configuration: {} },
         ],
       } as any);
-      expect(map).toEqual({ at: EFieldWidgetType.DatePicker, code: undefined });
+      expect(map).toEqual({ at: EFieldWidgetType.DatePicker, code: null });
     });
     it("returns an empty map when configuration is missing", () => {
       expect(getChangeWidgetTypes()).toEqual({});