Reject Unknown Status Filters, Do Not Ignore Them
A characterization test demanded total = 3 for status=bogus, so the API answered 200 with every live row. One switch statement flipped it to fail-closed 400.
TL;DR
The endpoint silently ignored unrecognized status filters, returning all rows with 200 OK, and a green test had pinned that fail-open behavior as intended. A strict switch now separates empty string from invalid values, rejecting the latter with 400. Guidelines like AIP-160 agree, proving green tests don't guarantee a correct contract.
I was reviewing the characterization test TestRepository_ListFilters for the list endpoint of the KotaPortal API, and one assertion looked wrong in a quiet way: querying with status=bogus returned total = 3. The endpoint answered 200 OK with every live row, as if the parameter had never been sent. The suite was green, and that was the problem.
The usual defense for this kind of code is that it is a tolerant filter: the server accepts what it can and moves on. My own first guess was the same, some deliberate leniency for older clients with stale query strings.
It was not leniency. It was filter-dropping: an unrecognized status value behaved exactly like an absent filter, so a typo produced a wrong result instead of an error. And the test suite had that decision pinned as intended behavior, because the old assertion literally demanded total = 3 for status=bogus.
The Fail-Open Trap
I replaced the loose if chain with an explicit switch: one arm for the empty value, one arm for the whitelist members, and a default arm that rejects.
switch q.Status {
case "":
// "" = tanpa filter status; ini kontrak tab "semua" di frontend
case "draft", "published", "scheduled":
where += " AND e.status = ?"
args = append(args, q.Status)
default:
return nil, 0, fmt.Errorf(
"%w: status harus draft, published, atau scheduled",
ErrValidation,
)
}The empty string is a value here, not missing input. The frontend has an "all" tab that really sends status= with nothing after it, so the strictness lives in the default arm instead of in a blanket rejection of everything outside the whitelist. "No value" and "invalid value" are two different states, and the code now separates them on purpose.
Before flipping the behavior I mapped the blast radius. Service.List is the only production caller of the repository List, and Handler.list is its only caller, so the decision stands at a single point. The compatibility cost is real: clients that used to send a misspelled status got 200 with the full list, and now they get 400. That is the point. A wrong list is more expensive than a fast error, because this kind of mistake only shows up after somebody has already misread the result.
The test went from red to green the honest way. The old assertion was replaced with errors.Is(err, ErrValidation), and I added two anchors so the contract does not drift back: List(ListQuery{}) still returns all live rows (total 3), and status=scheduled still filters (0 rows, since the one scheduled row is trashed). The error reuses the existing ErrValidation sentinel, so the handler only maps it to 400 VALIDATION with no new error path.
The Standards Point the Same Way
Silently ignoring input means the API lies about what it understood, and the official guidance sides with explicit rejection.
AIP-160 Filtering says field values for bounded data types such as enums in a filter "must be a valid value in the set", and a non-compliant or schematically invalid filter "should error with INVALID_ARGUMENT" [1]. The Azure REST API Guidelines are blunter: validate every query parameter and header value and fail the operation with 400 Bad Request as soon as one value fails validation, with an error response that explains what is wrong so the caller can fix it [2]. OWASP Input Validation Cheat Sheet frames the same rule as a security control: define what the application accepts and reject values outside those rules, instead of trying to recognize every malicious string one by one [3]. RFC 9110 defines 400 Bad Request as the status for a request the server cannot or will not process because of a perceived client error, invalid request syntax included [4]. A bogus status sits squarely in that category.
Which Side Gets to Be Tolerant
What usually gets swapped is the direction of the tolerance. The Azure guidelines split it in two: a service "may return" an extensible enum value that is not defined for the api-version in the request, but it "should not accept" such a value from the request itself [2]. Readers can be loose, writers have to be strict. A response carrying a status value from a newer server version is still readable by an older client; a filter sending an unknown value to the server is a different case, because there the client misunderstands the value domain.
The lesson I keep from this one: a green suite does not guarantee a correct contract. An assertion can enshrine a wrong fail-open decision, and the way to find out is to re-read the tests that lock in strange behavior and turn them red before touching the code.