Skip to content

core: CHECK constraint and service validation for row_limit_override >= 1 #1085

Description

@babltiga

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

  1. 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.
  2. 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).
  3. 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.
  4. Check the Terraform provider schema and the bootstrap spec for the same field, and add a >= 1 validator where they accept it.
  5. Tests: a migration IT (Testcontainers) proves the constraint rejects 0; unit tests cover the service validation and the defensive merge.
  6. 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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions