Add security.rs lower-bound limit tests for capped list routes (#431) #959

2026-06-29T15:41:30Z

PlasticDigits commented

2026-06-29 15:41:30 +00:00

(Migrated from gitlab.com)

Summary

GitLab #431 (SEC-F05): the seven capped list routes in indexer/tests/security.rs already had upper-bound tests (limit=99999) but no lower-bound coverage for limit=-1 or limit=0.

This MR adds seven integration tests (one per route) asserting the defined contract: negative/zero limit values clamp to 1 via .clamp(1, max) → HTTP 200 with at most one row (never a negative SQL LIMIT → 500, per #284). Docs and the LCD security skill cross-link the new tests.

Routes covered: pair trades, pair candles, oracle history, trader trades, limit-fills, limit-placements, limit-cancellations.

Acceptance checklist

Criterion	Verification	Result
`security.rs` has `limit=-1` test on all seven capped list routes	`cd indexer && cargo test --test security -j 1 -- --test-threads=1 pair_trades_negative pair_candles_negative oracle_history_negative trader_trades_negative limit_fills_negative limit_placements_negative limit_cancellations_negative`	PASS
`security.rs` has `limit=0` test on all seven capped list routes	Same command (each test sends both `-1` and `0`)	PASS
All seven routes covered: trades, candles, oracle/history, trader trades, limit-fills, limit-placements, limit-cancellations	Inspect `indexer/tests/security.rs` `*_negative_and_zero_limit_clamp_to_one` tests	PASS

Verification checklist (third parties)

make setup-indexer-postgres (or existing Postgres + indexer/.env)
cd indexer && cargo test --test security -j 1 -- --test-threads=1 — all 35 tests pass
Optional broader sweep: cd indexer && cargo test --test api_limit_lower_bound -j 1 -- --test-threads=1 — 16 tests pass (every SQL-backed list endpoint, #317)

Lower-bound contract documented in docs/indexer-invariants.md (Numeric query caps row)
Broader per-endpoint sweep remains in indexer/tests/api_limit_lower_bound.rs (#317)
Static guardrail: indexer/tests/limit_clamp_guardrail.rs

## Summary GitLab #431 (SEC-F05): the seven capped list routes in `indexer/tests/security.rs` already had upper-bound tests (`limit=99999`) but no lower-bound coverage for `limit=-1` or `limit=0`. This MR adds seven integration tests (one per route) asserting the defined contract: negative/zero `limit` values clamp to **1** via `.clamp(1, max)` → HTTP **200** with at most one row (never a negative SQL `LIMIT` → **500**, per #284). Docs and the LCD security skill cross-link the new tests. Routes covered: pair trades, pair candles, oracle history, trader trades, limit-fills, limit-placements, limit-cancellations. ## Acceptance checklist | Criterion | Verification | Result | |-----------|--------------|--------| | `security.rs` has `limit=-1` test on all seven capped list routes | `cd indexer && cargo test --test security -j 1 -- --test-threads=1 pair_trades_negative pair_candles_negative oracle_history_negative trader_trades_negative limit_fills_negative limit_placements_negative limit_cancellations_negative` | PASS | | `security.rs` has `limit=0` test on all seven capped list routes | Same command (each test sends both `-1` and `0`) | PASS | | All seven routes covered: trades, candles, oracle/history, trader trades, limit-fills, limit-placements, limit-cancellations | Inspect `indexer/tests/security.rs` `*_negative_and_zero_limit_clamp_to_one` tests | PASS | ## Verification checklist (third parties) 1. `make setup-indexer-postgres` (or existing Postgres + `indexer/.env`) 2. `cd indexer && cargo test --test security -j 1 -- --test-threads=1` — all 35 tests pass 3. Optional broader sweep: `cd indexer && cargo test --test api_limit_lower_bound -j 1 -- --test-threads=1` — 16 tests pass (every SQL-backed list endpoint, #317) ## Related context - Lower-bound contract documented in [`docs/indexer-invariants.md`](docs/indexer-invariants.md) (Numeric query caps row) - Broader per-endpoint sweep remains in [`indexer/tests/api_limit_lower_bound.rs`](indexer/tests/api_limit_lower_bound.rs) (#317) - Static guardrail: [`indexer/tests/limit_clamp_guardrail.rs`](indexer/tests/limit_clamp_guardrail.rs)

PlasticDigits commented

2026-06-29 15:43:32 +00:00

(Migrated from gitlab.com)

Security review — MR !959

Commit reviewed: 536ff00cc15846244262c2982094c555df448e21
Scope: indexer/tests/security.rs (7 lower-bound limit integration tests + shared helper), docs/indexer-invariants.md, skills/AGENTS_INDEXER_API_LCD_SECURITY.md
Outcome: FINDINGS: 0 medium+

Summary

Security review: no medium+ findings on this diff.

This MR is test-and-documentation only — no production indexer, CosmWasm, or frontend code changes. The added tests assert the existing .clamp(1, max) contract on seven capped list routes (limit=-1 / limit=0 → HTTP 200, at most one row) and align with the production handlers in pairs.rs, traders.rs, and oracle.rs where limit is bound before parameterized SQL.

Attack-path trace (reviewed routes): attacker-controlled limit query param → Option<i64> deserialize → .clamp(1, MAX) → i64 bind to SQL LIMIT. Negative/zero values cannot reach Postgres as invalid LIMIT (historical #284 class). No new injection surface, auth bypass, secret leakage, SSRF, or deserialization risk introduced by the diff.

Inline threads: none (no findings to anchor).

Prior security-review notes on this MR: none found.

## Security review — MR !959 **Commit reviewed:** `536ff00cc15846244262c2982094c555df448e21` **Scope:** `indexer/tests/security.rs` (7 lower-bound `limit` integration tests + shared helper), `docs/indexer-invariants.md`, `skills/AGENTS_INDEXER_API_LCD_SECURITY.md` **Outcome:** `FINDINGS: 0` medium+ ### Summary Security review: **no medium+ findings** on this diff. This MR is test-and-documentation only — no production indexer, CosmWasm, or frontend code changes. The added tests assert the existing `.clamp(1, max)` contract on seven capped list routes (`limit=-1` / `limit=0` → HTTP 200, at most one row) and align with the production handlers in `pairs.rs`, `traders.rs`, and `oracle.rs` where `limit` is bound before parameterized SQL. **Attack-path trace (reviewed routes):** attacker-controlled `limit` query param → `Option<i64>` deserialize → `.clamp(1, MAX)` → `i64` bind to SQL `LIMIT`. Negative/zero values cannot reach Postgres as invalid `LIMIT` (historical #284 class). No new injection surface, auth bypass, secret leakage, SSRF, or deserialization risk introduced by the diff. **Inline threads:** none (no findings to anchor). Prior security-review notes on this MR: none found.

PlasticDigits commented

2026-06-29 15:49:23 +00:00

(Migrated from gitlab.com)

mentioned in commit 55cf80af2e

mentioned in commit 55cf80af2e6f2f4b6e562b106fe9d99df6e5168a