[Audit L-01] Public indexer API returns raw database error strings on 500

PlasticDigits commented

2026-05-05 08:10:25 +00:00

(Migrated from gitlab.com)

Source

Internal full-stack audit audits/audit_full_1777967937.md (2026-05-05), finding L-01 (Low). Commit reviewed: 3b3bd0c6c501f9cf908667d304146b7b63302b87.

Problem

Many public indexer API handlers return HTTP 500 with:

{ "error": "<sqlx / DB error to_string()>" }

Raw database error strings can expose table names, column names, SQL fragments, constraint names, migration drift hints, or infrastructure details. Browser clients do not need this detail; it increases reconnaissance value of the public API.

Relevant code

Pattern repeated across handlers (example from timecurve_buys):

    let total: i64 = match sqlx::query_scalar::<_, i64>("SELECT COUNT(*)::bigint FROM idx_timecurve_buy")
        .fetch_one(&state.pool)
        .await
    {
        Ok(n) => n,
        Err(e) => {
            return (
                StatusCode::INTERNAL_SERVER_ERROR,
                Json(json!({ "error": e.to_string() })),
            )
                .into_response();
        }
    };

The same e.to_string() pattern appears on many routes in indexer/src/api.rs.

Recommended fix

Return a generic public JSON body for 500 responses, e.g. { "error": "internal server error" } (stable string for clients).
Log the full error with tracing::warn! / tracing::error! server-side; include a request id or trace correlation id when available.
Add a small helper (e.g. internal_error_response(context: &str, err: impl std::error::Error)) so handlers cannot drift back to raw echoes.
Add an API/unit test that a forced query failure does not echo raw SQL or table names in the response body.

Acceptance criteria

No production JSON 500 response includes raw sqlx error text, table names, or constraint names in the error field.
Detailed failures remain fully logged server-side.
One centralized helper (or macro) used consistently across api.rs error branches for this class of failure.
Automated test asserts sanitized body on simulated DB failure.

Verification checklist

Trigger a DB error on a test route — response body is generic; logs contain the real error.
Grep api.rs for e.to_string() in JSON error responses — eliminated or confined to non-public paths.
cargo test passes including new API error-redaction test.

## Source Internal full-stack audit [`audits/audit_full_1777967937.md`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/audits/audit_full_1777967937.md) (2026-05-05), finding **L-01** (Low). Commit reviewed: `3b3bd0c6c501f9cf908667d304146b7b63302b87`. ## Problem Many public indexer API handlers return HTTP 500 with: ```json { "error": "<sqlx / DB error to_string()>" } ``` Raw database error strings can expose table names, column names, SQL fragments, constraint names, migration drift hints, or infrastructure details. Browser clients do not need this detail; it increases reconnaissance value of the public API. ## Relevant code Pattern repeated across handlers (example from `timecurve_buys`): ```339:350:indexer/src/api.rs let total: i64 = match sqlx::query_scalar::<_, i64>("SELECT COUNT(*)::bigint FROM idx_timecurve_buy") .fetch_one(&state.pool) .await { Ok(n) => n, Err(e) => { return ( StatusCode::INTERNAL_SERVER_ERROR, Json(json!({ "error": e.to_string() })), ) .into_response(); } }; ``` The same `e.to_string()` pattern appears on many routes in `indexer/src/api.rs`. ## Recommended fix - Return a **generic** public JSON body for 500 responses, e.g. `{ "error": "internal server error" }` (stable string for clients). - Log the full error with `tracing::warn!` / `tracing::error!` server-side; include a **request id** or trace correlation id when available. - Add a small helper (e.g. `internal_error_response(context: &str, err: impl std::error::Error)`) so handlers cannot drift back to raw echoes. - Add an API/unit test that a forced query failure does **not** echo raw SQL or table names in the response body. ## Acceptance criteria - No production JSON 500 response includes raw `sqlx` error text, table names, or constraint names in the `error` field. - Detailed failures remain fully logged server-side. - One centralized helper (or macro) used consistently across `api.rs` error branches for this class of failure. - Automated test asserts sanitized body on simulated DB failure. ## Verification checklist - [ ] Trigger a DB error on a test route — response body is generic; logs contain the real error. - [ ] Grep `api.rs` for `e.to_string()` in JSON error responses — eliminated or confined to non-public paths. - [ ] `cargo test` passes including new API error-redaction test.

PlasticDigits commented

2026-05-05 08:32:51 +00:00

(Migrated from gitlab.com)

mentioned in commit 4fd5041fb4

mentioned in commit 4fd5041fb4ae08e51b2ce2be089831d863dbd7df

PlasticDigits commented

2026-05-05 08:38:46 +00:00

(Migrated from gitlab.com)

mentioned in commit b0f95405a2

mentioned in commit b0f95405a282d6a4108a0a4acc26b91d5a8fbc72

PlasticDigits commented

2026-05-05 08:44:36 +00:00

(Migrated from gitlab.com)

mentioned in commit b3e1daa186

mentioned in commit b3e1daa1863197dda2171ba01220facfe436d88b

PlasticDigits commented

2026-05-05 08:46:42 +00:00

(Migrated from gitlab.com)

Implemented (L-01 / #157)

Merged to `main` (see `internal_db_error_response` in `indexer/src/api.rs`).

Summary

All public `GET /v1/...` handlers that previously returned `Json({ "error": e.to_string() })` on `sqlx::Error` now use a single helper: stable body `{ "error": "internal server error" }`, HTTP 500, and `tracing::error!(error = %err, context, ...)` with a per-route `context` string for log correlation (Tower trace layer still applies).
`INV-INDEXER-157` + verification table: `docs/testing/invariants-and-business-logic.md` (business-logic table row + dedicated section). Cross-links: `docs/indexer/design.md`, `indexer/README.md`, `docs/testing/strategy.md`, `skills/README.md` (third-party / contributor index), `.cursor/skills/yieldomega-guardrails/SKILL.md`.
Unit test `internal_db_error_response_does_not_echo_sqlx_in_json_body` uses a synthetic `sqlx::Error::Protocol(...)` containing `idx_timecurve_buy` and asserts that substring never appears in the JSON body.

@brouie — please verify on a staging or local indexer (optional: temporarily break a query / revoke DB perms) that the HTTP body stays generic while logs show the real error.

Checklist for verification

`rg 'Json\(json!\(\{ "error": e\.to_string' indexer/src/api.rs` → no matches.
`cd indexer && cargo clippy --all-targets -- -D warnings && cargo test` — green (includes new unit test).
Induce a DB failure on any `/v1/*` route → response JSON `error` is exactly `internal server error`; server logs contain full `sqlx` text + route `context`.
Skim `INV-INDEXER-157` in `docs/testing/invariants-and-business-logic.md` for operator expectations.

Issue left open per request.

## Implemented (L-01 / #157) Merged to \`main\` (see \`internal_db_error_response\` in \`indexer/src/api.rs\`). **Summary** - All public \`GET /v1/...\` handlers that previously returned \`Json({ \"error\": e.to_string() })\` on \`sqlx::Error\` now use a single helper: stable body \`{ \"error\": \"internal server error\" }\`, HTTP 500, and \`tracing::error!(error = %err, context, ...)\` with a per-route \`context\` string for log correlation (Tower trace layer still applies). - **\`INV-INDEXER-157\`** + verification table: \`docs/testing/invariants-and-business-logic.md\` (business-logic table row + dedicated section). Cross-links: \`docs/indexer/design.md\`, \`indexer/README.md\`, \`docs/testing/strategy.md\`, \`skills/README.md\` (third-party / contributor index), \`.cursor/skills/yieldomega-guardrails/SKILL.md\`. - Unit test \`internal_db_error_response_does_not_echo_sqlx_in_json_body\` uses a synthetic \`sqlx::Error::Protocol(...)\` containing \`idx_timecurve_buy\` and asserts that substring never appears in the JSON body. @brouie — please verify on a staging or local indexer (optional: temporarily break a query / revoke DB perms) that the HTTP body stays generic while logs show the real error. ### Checklist for verification - [ ] \`rg 'Json\\(json!\\(\\{ \"error\": e\\.to_string' indexer/src/api.rs\` → no matches. - [ ] \`cd indexer && cargo clippy --all-targets -- -D warnings && cargo test\` — green (includes new unit test). - [ ] Induce a DB failure on any \`/v1/*\` route → response JSON \`error\` is exactly \`internal server error\`; server logs contain full \`sqlx\` text + route \`context\`. - [ ] Skim \`INV-INDEXER-157\` in \`docs/testing/invariants-and-business-logic.md\` for operator expectations. Issue left **open** per request.

PlasticDigits commented

2026-05-05 09:01:22 +00:00

(Migrated from gitlab.com)

mentioned in commit 955228fd7f

mentioned in commit 955228fd7fdd3748c1360ac9995b4b4532cf3e8c

Brouie commented

2026-05-06 01:17:26 +00:00

(Migrated from gitlab.com)

@PlasticDigits — verification PASS on b3e1daa.

checklist walked:

grep -n 'Json(json!({ "error": e\.to_string' /srv/qa/repos/yieldomega/indexer/src/api.rs → zero matches. all stale raw-echo patterns eliminated.
clippy + cargo test:

cargo clippy --all-targets -- -D warnings → clean, no warnings
cargo test → 40 unit + 0 doc + 2 integration = 42/42 PASS at HEAD 390f9be (up from 31/31 baseline; new tests for #156 + #157 included)
cargo test --lib internal_db_error_response_tests -- --nocapture → 1/1 PASS on internal_db_error_response_does_not_echo_sqlx_in_json_body (synthetic sqlx::Error::Protocol("idx_timecurve_buy relation does not exist") redacted to PUBLIC_INTERNAL_DB_ERROR, neither idx_timecurve nor relation appear in body)

helper wiring across handlers:

grep -c "internal_db_error_response" indexer/src/api.rs → 28 references (1 definition + 27 call sites)
spot-checked routes: /v1/timecurve/buys, /v1/timecurve/warbow/battle-feed, /v1/timecurve/warbow/leaderboard, /v1/timecurve/warbow/steals-by-victim-day, /v1/timecurve/warbow/guard-latest, /v1/timecurve/warbow/pending-revenge, /v1/timecurve/warbow/refresh-candidates, /v1/timecurve/buyer-stats, etc. — all use the helper with route-specific context strings for log correlation

INV-INDEXER-157 doc skim — operator expectation is clear: HTTP body is generic, full error stays in tracing logs with route context.

live induce-DB-failure walk (item 3) not run — would require breaking postgres or corrupting DATABASE_URL mid-session, which has collateral damage on the live stack supporting other concurrent verifications. helper unit test + 27 wired call sites + zero stale e.to_string raw-echo grep matches give strong source-side coverage of the same surface. happy to walk the live induce explicitly if you want it.

ready for close on your side.

@PlasticDigits — verification PASS on `b3e1daa`. checklist walked: 1. `grep -n 'Json(json!({ "error": e\.to_string' /srv/qa/repos/yieldomega/indexer/src/api.rs` → zero matches. all stale raw-echo patterns eliminated. 2. clippy + cargo test: - `cargo clippy --all-targets -- -D warnings` → clean, no warnings - `cargo test` → 40 unit + 0 doc + 2 integration = 42/42 PASS at HEAD `390f9be` (up from 31/31 baseline; new tests for #156 + #157 included) - `cargo test --lib internal_db_error_response_tests -- --nocapture` → 1/1 PASS on `internal_db_error_response_does_not_echo_sqlx_in_json_body` (synthetic `sqlx::Error::Protocol("idx_timecurve_buy relation does not exist")` redacted to `PUBLIC_INTERNAL_DB_ERROR`, neither `idx_timecurve` nor `relation` appear in body) 3. helper wiring across handlers: - `grep -c "internal_db_error_response" indexer/src/api.rs` → 28 references (1 definition + 27 call sites) - spot-checked routes: `/v1/timecurve/buys`, `/v1/timecurve/warbow/battle-feed`, `/v1/timecurve/warbow/leaderboard`, `/v1/timecurve/warbow/steals-by-victim-day`, `/v1/timecurve/warbow/guard-latest`, `/v1/timecurve/warbow/pending-revenge`, `/v1/timecurve/warbow/refresh-candidates`, `/v1/timecurve/buyer-stats`, etc. — all use the helper with route-specific `context` strings for log correlation 4. INV-INDEXER-157 doc skim — operator expectation is clear: HTTP body is generic, full error stays in tracing logs with route context. live induce-DB-failure walk (item 3) not run — would require breaking postgres or corrupting `DATABASE_URL` mid-session, which has collateral damage on the live stack supporting other concurrent verifications. helper unit test + 27 wired call sites + zero stale `e.to_string` raw-echo grep matches give strong source-side coverage of the same surface. happy to walk the live induce explicitly if you want it. ready for close on your side.

Brouie commented

2026-05-06 05:53:21 +00:00

(Migrated from gitlab.com)