Expected behavior and actual behavior:
Expected: when a q= filter operand cannot be parsed as the type of the column it targets, the API should reject the request with 400 BadRequest and name the offending parameter, the way other malformed query input is handled.
Actual: the operand is handed to Postgres verbatim, the driver rejects it, and the request fails with 500 and a generic UNKNOWN error. The driver message is written to the core log.
Here is what I get on a 2.15.x deployment. All requests are GET against /api/v2.0, authenticated as admin:
| request |
status |
response body |
projects?q=creation_time=[a~b] |
500 |
{"errors":[{"code":"UNKNOWN","message":"internal server error"}]} |
projects?q=creation_time=abc |
500 |
same |
projects?q=creation_time={a b} |
500 |
same |
projects?q=project_id=abc |
500 |
same |
projects?q=project_id=[a~b] |
500 |
same |
audit-logs?q=op_time=[x~y] |
500 |
same |
repositories?q=creation_time=abc |
500 |
same |
users?q=creation_time=abc |
500 |
same |
registries?q=creation_time=abc |
500 |
same |
replication/policies?q=creation_time=abc |
500 |
same |
system/purgeaudit?q=update_time=abc |
500 |
same |
Well-formed operands behave correctly, which rules out the filter keys themselves being at fault:
| request |
status |
projects?q=creation_time=[2020-01-01~2021-01-01] |
200 |
projects?q=creation_time=~abc |
200 |
The fuzzy-match case survives because it compiles to ILIKE, and Postgres casts the column to text before comparing, so the operand never has to parse as a timestamp.
Two things make this more than cosmetic. The 500 is reachable without credentials on any endpoint that permits anonymous access, so an unauthenticated client can drive an instance's 5xx rate at will:
curl -s -w '\nHTTP %{http_code}\n' -G http://harbor.example.com/api/v2.0/projects \
--data-urlencode 'q=creation_time=abc'
{"errors":[{"code":"UNKNOWN","message":"internal server error"}]}
HTTP 500
And because these land as genuine server errors, they pollute error-rate dashboards and page on-call for what is really a client typo.
Steps to reproduce the problem:
Any Harbor backed by Postgres reproduces this. Substitute your own credentials and hostname.
HARBOR=http://harbor.example.com/api/v2.0
AUTH='admin:<password>'
# fails with 500
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/projects" --data-urlencode 'q=creation_time=[a~b]'
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/projects" --data-urlencode 'q=creation_time=abc'
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/projects" --data-urlencode 'q=creation_time={a b}'
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/projects" --data-urlencode 'q=project_id=abc'
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/projects" --data-urlencode 'q=project_id=[a~b]'
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/audit-logs" --data-urlencode 'q=op_time=[x~y]'
# succeeds with 200
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/projects" --data-urlencode 'q=creation_time=[2020-01-01~2021-01-01]'
curl -s -u "$AUTH" -w '\nHTTP %{http_code}\n' -G "$HARBOR/projects" --data-urlencode 'q=creation_time=~abc'
Versions:
- harbor version: reproduced on a 2.15.8 build; the code path described below is unchanged on
main as of 3488b6455a7a8cde5cbdf92b243b54567b9e61a9
- docker engine version: podman 6.1.1
- docker-compose version: podman-compose 1.6.0
- postgres version: 16.15
Additional context:
Core log for the six requests in the first block, in order:
[ERROR] [/lib/http/error.go:58]: {"errors":[{"code":"UNKNOWN","message":"unknown: ERROR: invalid input syntax for type timestamp: \"a\" (SQLSTATE 22007)"}]}
[ERROR] [/lib/http/error.go:58]: {"errors":[{"code":"UNKNOWN","message":"unknown: ERROR: invalid input syntax for type timestamp: \"abc\" (SQLSTATE 22007)"}]}
[ERROR] [/lib/http/error.go:58]: {"errors":[{"code":"UNKNOWN","message":"unknown: ERROR: invalid input syntax for type timestamp: \"a\" (SQLSTATE 22007)"}]}
[ERROR] [/lib/http/error.go:58]: {"errors":[{"code":"UNKNOWN","message":"unknown: ERROR: invalid input syntax for type integer: \"abc\" (SQLSTATE 22P02)"}]}
[ERROR] [/lib/http/error.go:58]: {"errors":[{"code":"UNKNOWN","message":"unknown: ERROR: invalid input syntax for type integer: \"a\" (SQLSTATE 22P02)"}]}
[ERROR] [/lib/http/error.go:58]: {"errors":[{"code":"UNKNOWN","message":"unknown: ERROR: invalid input syntax for type timestamp: \"x\" (SQLSTATE 22007)"}]}
SendError in src/lib/http/error.go replaces the payload with a generic message for any status at or above 500, which is why the driver text appears in the log and not in the response.
Root cause looks like setFilters in src/lib/orm/query.go. Once a keyword has been resolved to a filterable field, the value is a plain string taken from the parsed q= expression, and it goes straight onto the beego QuerySeter with no check against the type of the model field it targets:
- the range branch at lines 205-213 applies
r.Min and r.Max as-is
- the or-list branch at lines 215-222 applies
ol.Values as-is
- the exact-match fallthrough at line 229 applies
value as-is
beego assembles the SQL happily, and Postgres is the first component in the chain with an opinion about the type. By then the failure is an opaque driver error that nothing upstream classifies as a client error, so it falls through to GeneralCode and 500.
This shape of bug has been fixed twice at the handler layer, in #23670 for robot accounts and in #23669 for /system/purgeaudit. Both hardened type assertions inside a single handler, so neither covers this path. Every endpoint that builds its query through orm.QuerySetter still has the problem, which is why the matrix above spans projects, repositories, audit logs, users, registries and replication policies. Fixing it once in the ORM layer would cover all of them and remove the need to patch handlers one at a time.
We carry a fix in a downstream fork that validates the operand against the model field type inside the ORM layer and returns BadRequestError naming the parameter, for example invalid value for the query parameter "creation_time": abc. It is deliberately permissive, so anything that resolves today still resolves, including the ILIKE fuzzy path and filters served by a custom FilterFunc. Happy to open a PR against main if that approach sounds acceptable, or to rework it if you would rather see the validation live somewhere else.
Expected behavior and actual behavior:
Expected: when a
q=filter operand cannot be parsed as the type of the column it targets, the API should reject the request with400 BadRequestand name the offending parameter, the way other malformed query input is handled.Actual: the operand is handed to Postgres verbatim, the driver rejects it, and the request fails with
500and a genericUNKNOWNerror. The driver message is written to the core log.Here is what I get on a 2.15.x deployment. All requests are
GETagainst/api/v2.0, authenticated as admin:projects?q=creation_time=[a~b]{"errors":[{"code":"UNKNOWN","message":"internal server error"}]}projects?q=creation_time=abcprojects?q=creation_time={a b}projects?q=project_id=abcprojects?q=project_id=[a~b]audit-logs?q=op_time=[x~y]repositories?q=creation_time=abcusers?q=creation_time=abcregistries?q=creation_time=abcreplication/policies?q=creation_time=abcsystem/purgeaudit?q=update_time=abcWell-formed operands behave correctly, which rules out the filter keys themselves being at fault:
projects?q=creation_time=[2020-01-01~2021-01-01]projects?q=creation_time=~abcThe fuzzy-match case survives because it compiles to
ILIKE, and Postgres casts the column to text before comparing, so the operand never has to parse as a timestamp.Two things make this more than cosmetic. The 500 is reachable without credentials on any endpoint that permits anonymous access, so an unauthenticated client can drive an instance's 5xx rate at will:
And because these land as genuine server errors, they pollute error-rate dashboards and page on-call for what is really a client typo.
Steps to reproduce the problem:
Any Harbor backed by Postgres reproduces this. Substitute your own credentials and hostname.
Versions:
mainas of3488b6455a7a8cde5cbdf92b243b54567b9e61a9Additional context:
Core log for the six requests in the first block, in order:
SendErrorinsrc/lib/http/error.goreplaces the payload with a generic message for any status at or above 500, which is why the driver text appears in the log and not in the response.Root cause looks like
setFiltersinsrc/lib/orm/query.go. Once a keyword has been resolved to a filterable field, the value is a plain string taken from the parsedq=expression, and it goes straight onto the beegoQuerySeterwith no check against the type of the model field it targets:r.Minandr.Maxas-isol.Valuesas-isvalueas-isbeego assembles the SQL happily, and Postgres is the first component in the chain with an opinion about the type. By then the failure is an opaque driver error that nothing upstream classifies as a client error, so it falls through to
GeneralCodeand 500.This shape of bug has been fixed twice at the handler layer, in #23670 for robot accounts and in #23669 for
/system/purgeaudit. Both hardened type assertions inside a single handler, so neither covers this path. Every endpoint that builds its query throughorm.QuerySetterstill has the problem, which is why the matrix above spans projects, repositories, audit logs, users, registries and replication policies. Fixing it once in the ORM layer would cover all of them and remove the need to patch handlers one at a time.We carry a fix in a downstream fork that validates the operand against the model field type inside the ORM layer and returns
BadRequestErrornaming the parameter, for exampleinvalid value for the query parameter "creation_time": abc. It is deliberately permissive, so anything that resolves today still resolves, including theILIKEfuzzy path and filters served by a customFilterFunc. Happy to open a PR againstmainif that approach sounds acceptable, or to rework it if you would rather see the validation live somewhere else.