Goal
Enforce row_limit_override >= 1 at every layer, not just at the web layer, so a bad value cannot reach the proxy.
What is true today
- The only guard is
@Min(1) on CreatePermissionRequest / CreateGroupPermissionRequest (security/internal/web/model).
- The columns have no CHECK constraint:
V4__create_permissions.sql:8 and V111__create_group_access_permissions.sql:16.
DatasourceAdminServiceImpl (:642, :696) persists command.rowLimitOverride() without validating it.
Since #933 (PR #1083) the value is read at execution time. QueryExecutionRequest rejects maxRowsOverride <= 0 with an IllegalArgumentException. A row holding 0 or a negative value therefore makes every query that user runs on that datasource fail with a confusing internal error (the query is recorded FAILED). This fails closed, so nothing leaks, but it's a support trap. Such a row can come from a path that skips the web DTO: a future service caller, bootstrap/IaC, SCIM-driven grants, or a manual SQL fix.
Steps
- Flyway migration
V{next}__row_limit_override_positive.sql:
- First, normalise any existing non-positive values to
NULL (meaning "use the datasource cap"). Otherwise the constraint cannot be added on a dirty table.
- Then add
CHECK (row_limit_override IS NULL OR row_limit_override >= 1) on both datasource_user_permissions and datasource_group_permissions.
- Never edit V4/V111.
- Service-level validation in
DatasourceAdminServiceImpl for both create/update paths. Throw the module's existing validation exception with an i18n key in all six messages*.properties files (MessagesParityTest).
- Defensive read. In
DefaultDatasourceUserPermissionLookupService, treat a non-positive stored value as absent when merging, and log a WARN. The merge then never hands the proxy an invalid override, even before the migration runs.
- Check the Terraform provider schema and the bootstrap spec for the same field, and add a
>= 1 validator where they accept it.
- Tests: a migration IT (Testcontainers) proves the constraint rejects
0; unit tests cover the service validation and the defensive merge.
- Docs:
docs/03-data-model.md (both tables).
Acceptance
- Inserting or updating
row_limit_override = 0 fails at the database.
- The admin service rejects it with a localized 400 regardless of caller.
- A pre-existing bad row no longer breaks the user's queries; it behaves like "no override".
Follow-up to #933.
Goal
Enforce
row_limit_override >= 1at every layer, not just at the web layer, so a bad value cannot reach the proxy.What is true today
@Min(1)onCreatePermissionRequest/CreateGroupPermissionRequest(security/internal/web/model).V4__create_permissions.sql:8andV111__create_group_access_permissions.sql:16.DatasourceAdminServiceImpl(:642,:696) persistscommand.rowLimitOverride()without validating it.Since #933 (PR #1083) the value is read at execution time.
QueryExecutionRequestrejectsmaxRowsOverride <= 0with anIllegalArgumentException. A row holding0or a negative value therefore makes every query that user runs on that datasource fail with a confusing internal error (the query is recordedFAILED). This fails closed, so nothing leaks, but it's a support trap. Such a row can come from a path that skips the web DTO: a future service caller, bootstrap/IaC, SCIM-driven grants, or a manual SQL fix.Steps
V{next}__row_limit_override_positive.sql:NULL(meaning "use the datasource cap"). Otherwise the constraint cannot be added on a dirty table.CHECK (row_limit_override IS NULL OR row_limit_override >= 1)on bothdatasource_user_permissionsanddatasource_group_permissions.DatasourceAdminServiceImplfor both create/update paths. Throw the module's existing validation exception with an i18n key in all sixmessages*.propertiesfiles (MessagesParityTest).DefaultDatasourceUserPermissionLookupService, treat a non-positive stored value as absent when merging, and log aWARN. The merge then never hands the proxy an invalid override, even before the migration runs.>= 1validator where they accept it.0; unit tests cover the service validation and the defensive merge.docs/03-data-model.md(both tables).Acceptance
row_limit_override = 0fails at the database.Follow-up to #933.