Skip to content

fix(filter): reject unknown filter operators instead of silently dropping the filter#283

Open
SAY-5 wants to merge 1 commit intoblnkfinance:mainfrom
SAY-5:fix/validate-filter-operator
Open

fix(filter): reject unknown filter operators instead of silently dropping the filter#283
SAY-5 wants to merge 1 commit intoblnkfinance:mainfrom
SAY-5:fix/validate-filter-operator

Conversation

@SAY-5
Copy link
Copy Markdown

@SAY-5 SAY-5 commented Apr 28, 2026

Summary

Closes #282.

When a JSON request body specifies a filter with an unrecognized operator
(e.g. \"equals\" instead of \"eq\"), the filter package was silently
dropping that filter and running the query unfiltered. Endpoints that
return the first matching row therefore looked successful even though the
user's filter never reached the database.

The reproduction in the issue is exactly this:

{ \"filters\": [ { \"field\": \"meta_data.name\", \"operator\": \"equals\", \"value\": \"...\" } ] }

The body parses successfully (operator is just a string), Validate
checks logical_operator and field but not the per-filter operator, and
buildStandardCondition falls through its default branch returning an
empty condition. Build skips empty conditions, so the resulting SQL has
no WHERE clause for that filter — the API returns the first row it finds
with no error.

Fix

  • Add IsValidOperator in internal/filter/operators.go, the
    operator-side counterpart to the existing IsValidLogicalOperator.
  • Reject unknown operators in Validate with a descriptive error that
    lists the supported operators (eq, ne, gt, gte, lt, lte,
    in, between, like, ilike, isnull, isnotnull).
  • This propagates as a 400 Bad Request from any endpoint that calls
    Build / BuildWithOptions, matching the existing handling of invalid
    fields and invalid logical operators.

The fix is minimal and additive — no signatures changed, no behavior
changes for valid operators, no new error categories.

Test plan

  • New unit tests cover unknown operator (\"equals\") and empty
    operator on both `Validate` and `Build`.
  • New `TestIsValidOperator` covers the operator-validation predicate.
  • Existing tests still pass (`go test ./internal/filter/... ./api/...`).
  • Verified regression: stashing the `Validate` change makes the new
    tests fail with the silent-drop behavior the issue describes.
  • `go vet ./internal/filter/...` and `gofmt -l internal/filter/` clean.

…ping the filter

Previously, when a request body included a filter with an unrecognized
operator (e.g. `equals` instead of `eq`), `Validate` did not check the
per-filter operator and `buildStandardCondition` fell through to its
`default` branch, returning an empty condition string. The caller then
skipped that condition entirely, leaving the query unfiltered. Endpoints
that fetch the first matching row would happily return whatever row they
saw first, making the response look successful even though the user's
filter was never applied.

This adds:

- `IsValidOperator` in `internal/filter/operators.go`, the operator-side
  counterpart to the existing `IsValidLogicalOperator`.
- A check in `Validate` that rejects unknown operators with a descriptive
  error listing the supported operators.
- Tests covering the unknown-operator and empty-operator cases on both
  `Validate` and `Build`, including a regression note tied to the
  reported issue.

Closes blnkfinance#282
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 28, 2026

CLA assistant check
All committers have signed the CLA.

@jerry-enebeli
Copy link
Copy Markdown
Collaborator

Thank you for the PR @SAY-5

@jerry-enebeli
Copy link
Copy Markdown
Collaborator

CLA assistant check Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

hi @SAY-5

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Filter query on DB returns results with bad operator

3 participants