Skip to content

fix(files): validate the list limit query parameter - #10673

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.50from
pacocartones:fix/files-limit-validation-verify
Aug 20, 2026
Merged

diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.50from
pacocartones:fix/files-limit-validation-verify

Conversation

@pacocartones

Copy link
Copy Markdown
Contributor

Summary

  • Validate the limit query parameter accepted by GET /v1/files.
  • Return a structured 400 invalid_request_error response for non-integer, zero, negative, or oversized values.
  • Preserve the existing default limit of 20 and maximum of 10,000.

Related Issues

  • No linked issue.

Validation

CI will run the repository gates on this PR.

  • Change type: other — Files API validation
  • Focused tests and category gates from the golden path
  • ESLint and Prettier checks on the changed files
  • Reconciled with the current active release base; focused checks rerun afterward
  • Production-code changes include a new automated test in this PR

Focused validation:

tests 38
pass 38
fail 0

The focused run covered:

  • tests/integration/files-api-limit-validation.test.ts
  • tests/integration/files-api.test.ts
  • tests/unit/batch_api.test.ts

Tests Added Or Updated

  • Added tests/integration/files-api-limit-validation.test.ts.

The test covers valid parsing, invalid limits, HTTP pagination behavior, and the HTTP 400 response.

Coverage Notes

The new integration test exercises both the query parser and the GET /v1/files route. The existing Files API and batch API suites also pass against this change.

Reviewer Notes

  • No database migrations or feature flags.
  • The maximum accepted value remains 10,000.

@pacocartones
pacocartones force-pushed the fix/files-limit-validation-verify branch from 648eb05 to 5cf9677 Compare August 18, 2026 21:28
@diegosouzapw

Copy link
Copy Markdown
Owner

Thanks for this fix, @pacocartones — the base handler lets a negative limit (e.g. ?limit=-5) pass Math.min(parseInt(...)||20, 10000) unchanged, and a negative LIMIT ? in SQLite is unbounded (reachable today via omniroute files list --limit -5), so validating the param closes a real gap.

Items to confirm before merge (no rework needed if already covered by the diff):

  1. Tests: please confirm the PR includes route-level coverage for the limit param (missing→default 20, 0/negative→rejected, non-integer/NaN→rejected, >10000→rejected/clamped, and the old unbounded-LIMIT regression). No route-level test for /v1/files exists today, so this is the main gate (Hard Rule fix(ci): add environment for npm token access #18).
  2. Error shape: if invalid input returns a 400, keep it consistent with this file's existing OpenAI-style { error: { message, type } } body (as POST already does), a static message (no raw param echo / no err.stack), with CORS_HEADERS.
  3. Nice-to-have (separate follow-up, not blocking): bin/cli/commands/files.mjs also passes the user's --limit through unclamped — worth the same guard client-side so files list --limit -5 surfaces a clean error.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw
diegosouzapw merged commit d9cb4f5 into diegosouzapw:release/v3.8.50 Aug 20, 2026
5 checks passed
diegosouzapw pushed a commit that referenced this pull request Aug 20, 2026
…ols (#10668)

Obrigado — PR muito bem documentado e verificado. Adiciona o gateway TabiToken (Anthropic-first, /v1/messages, x-api-key) e estende hcnsec de 1 para 4 protocolos (Chat, Responses, Anthropic Messages, Gemini). AlternateFormat ganha o hook urlBuilder opcional (necessário para o path model-scoped do Gemini), compartilhado com o provider gemini nativo em vez de duplicado.

Reconciliado nesta sessão contra o release tip atualizado (base drift real: 343→345 canônicos entre quando o PR foi criado e o merge, mais os PRs #10673/#10658 mergeados nesse meio-tempo). Conflitos em contagens de providers (docs, file-size baseline, teste de partição) resolvidos additivamente.

Validação (reconciliação a partir de origin/release/v3.8.50):
- typecheck:core limpo, complexity/cognitive-complexity dentro do baseline
- npm run check:provider-consistency — OK (266 REGISTRY entries, 346 providers canônicos, 0 exceções)
- 40/40 testes passando (newapi-gateway-providers, hcnsec-provider, providers-constants-split, alternate-formats)
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
Obrigado — bug real: GET /v1/files aceitava limit negativo sem validação (`-5 || 20` avalia truthy em -5, então Math.min(-5, 10000) = -5 passava direto). Agora valida integer/positivo/tamanho e retorna 400 estruturado para valores inválidos, preservando o default 20 e o máximo 10.000.

Validação (worktree combinado a partir de origin/release/v3.8.50, 0 conflitos):
- typecheck:core limpo, complexity/cognitive-complexity dentro do baseline
- tests/integration/files-api-limit-validation.test.ts — 5/5 passando
- tests/integration/files-api.test.ts — 12/12 passando (sem regressão)
- tests/unit/batch_api.test.ts teve 1 falha, confirmada DRIFT pré-existente idêntica no tip puro do release (não relacionada, timing de cancelamento de batch)
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…ols (diegosouzapw#10668)

Obrigado — PR muito bem documentado e verificado. Adiciona o gateway TabiToken (Anthropic-first, /v1/messages, x-api-key) e estende hcnsec de 1 para 4 protocolos (Chat, Responses, Anthropic Messages, Gemini). AlternateFormat ganha o hook urlBuilder opcional (necessário para o path model-scoped do Gemini), compartilhado com o provider gemini nativo em vez de duplicado.

Reconciliado nesta sessão contra o release tip atualizado (base drift real: 343→345 canônicos entre quando o PR foi criado e o merge, mais os PRs diegosouzapw#10673/diegosouzapw#10658 mergeados nesse meio-tempo). Conflitos em contagens de providers (docs, file-size baseline, teste de partição) resolvidos additivamente.

Validação (reconciliação a partir de origin/release/v3.8.50):
- typecheck:core limpo, complexity/cognitive-complexity dentro do baseline
- npm run check:provider-consistency — OK (266 REGISTRY entries, 346 providers canônicos, 0 exceções)
- 40/40 testes passando (newapi-gateway-providers, hcnsec-provider, providers-constants-split, alternate-formats)
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.

2 participants