Add lint-log-secrets guard against indexer secret logging (#433) #961

Merged

Brouie merged 1 commit from issue-433-lint-log-secrets into main

2026-06-29 16:21:40 +00:00

Brouie commented

2026-06-29 15:53:38 +00:00

(Migrated from gitlab.com)

SEC-F13 / #433: the indexer doesn't log secrets today, but nothing enforced it going forward.

What this adds

scripts/check_indexer_log_secrets.py — scans indexer/src/**/*.rs for logging/print macros (tracing::{info,warn,error,debug,trace}!, log::*, println!/eprintln!/dbg!) whose arguments reference a secret-bearing identifier (database_url, password, mnemonic, private_key, bearer, api_key, secret, credential, ...), or that log the whole Config struct via Debug. Config (config.rs) derives Debug and holds database_url (a postgres://user:password@... string), so {:?} on the whole struct would leak the password — both cases are caught.
make lint-log-secrets target, wired into make lint.

Deliberately does NOT flag bare token (the indexer is full of trading-token references) or config.field access (only the whole-struct Debug). A proven-safe line can opt out with a trailing // log-secrets-ok: <reason>.

Verified

Passes clean on current main: the four startup tracing::info! calls log only run_mode / lcd_urls / factory_address / rate-limits; database_url is used at connect() only. Also scanned the live indexer log (~15h) — zero postgres:// strings, zero password leaks.
Adversarially tested: planting tracing::info!("{}", config.database_url) and tracing::debug!("{:?}", config) are both flagged (exit 1); benign config.rate_limit_rps, a trading ask_token, and an opt-out line are correctly NOT flagged.

For #433.

SEC-F13 / #433: the indexer doesn't log secrets today, but nothing enforced it going forward. ## What this adds - `scripts/check_indexer_log_secrets.py` — scans `indexer/src/**/*.rs` for logging/print macros (`tracing::{info,warn,error,debug,trace}!`, `log::*`, `println!`/`eprintln!`/`dbg!`) whose arguments reference a secret-bearing identifier (`database_url`, `password`, `mnemonic`, `private_key`, `bearer`, `api_key`, `secret`, `credential`, ...), **or** that log the whole `Config` struct via `Debug`. `Config` (`config.rs`) derives `Debug` and holds `database_url` (a `postgres://user:password@...` string), so `{:?}` on the whole struct would leak the password — both cases are caught. - `make lint-log-secrets` target, wired into `make lint`. Deliberately does NOT flag bare `token` (the indexer is full of trading-token references) or `config.field` access (only the whole-struct Debug). A proven-safe line can opt out with a trailing `// log-secrets-ok: <reason>`. ## Verified - Passes clean on current `main`: the four startup `tracing::info!` calls log only `run_mode` / `lcd_urls` / `factory_address` / rate-limits; `database_url` is used at `connect()` only. Also scanned the live indexer log (~15h) — zero `postgres://` strings, zero password leaks. - Adversarially tested: planting `tracing::info!("{}", config.database_url)` and `tracing::debug!("{:?}", config)` are both flagged (exit 1); benign `config.rate_limit_rps`, a trading `ask_token`, and an opt-out line are correctly NOT flagged. For #433.

Brouie commented

2026-06-29 15:54:11 +00:00

(Migrated from gitlab.com)

mentioned in issue #433

PlasticDigits commented

2026-06-29 15:57:05 +00:00

(Migrated from gitlab.com)

Security review — MR !961

Commit reviewed: f6b871bd2e5ffd9454aa90ca73f14a69f60a1156
Scope: scripts/check_indexer_log_secrets.py (new), Makefile (lint-log-secrets target + lint dependency)
Outcome: FINDINGS: 0 medium+ — no inline threads.

Summary

Static-analysis guard for indexer secret logging (SEC-F13 / #433). Reviewed added/modified code and traced whether it introduces exploitable runtime attack surface or weakens existing controls.

Script (check_indexer_log_secrets.py):

Reads only hardcoded indexer/src/**/*.rs via Path.rglob; no attacker-controlled paths, shell execution, network I/O, or deserialization.
Regex-based macro-argument scan + optional // log-secrets-ok opt-out (visible in MR diffs; requires merge, not a runtime bypass).
Failure output prints source snippets only (no runtime secret values).

Makefile: Adds make lint-log-secrets and wires it into aggregate make lint; no injection surface.

Effectiveness (informational, below reporting threshold): Adversarial probes show expected static-analysis limits (e.g. aliased database_url, renamed cfg + {:?}, serde_json::to_string(&config) not flagged). These are pre-merge developer mistakes, not new attacker-controlled sinks; the guard adds defense-in-depth and does not replace authn/authz or remove existing controls. CI does not yet invoke lint-log-secrets (only make lint locally) — enforcement gap, not a vulnerability introduced by this diff.

Indexer runtime (unchanged by MR): Startup logging already limits fields to non-secret config (run_mode, lcd_urls, factory_address, rate limits); database_url used at connect() only.

Security review: no medium+ findings on this diff. No block:security label applied.

## Security review — MR !961 **Commit reviewed:** `f6b871bd2e5ffd9454aa90ca73f14a69f60a1156` **Scope:** `scripts/check_indexer_log_secrets.py` (new), `Makefile` (`lint-log-secrets` target + `lint` dependency) **Outcome:** **FINDINGS: 0** medium+ — no inline threads. ### Summary Static-analysis guard for indexer secret logging (SEC-F13 / #433). Reviewed added/modified code and traced whether it introduces exploitable runtime attack surface or weakens existing controls. **Script (`check_indexer_log_secrets.py`):** - Reads only hardcoded `indexer/src/**/*.rs` via `Path.rglob`; no attacker-controlled paths, shell execution, network I/O, or deserialization. - Regex-based macro-argument scan + optional `// log-secrets-ok` opt-out (visible in MR diffs; requires merge, not a runtime bypass). - Failure output prints source snippets only (no runtime secret values). **Makefile:** Adds `make lint-log-secrets` and wires it into aggregate `make lint`; no injection surface. **Effectiveness (informational, below reporting threshold):** Adversarial probes show expected static-analysis limits (e.g. aliased `database_url`, renamed `cfg` + `{:?}`, `serde_json::to_string(&config)` not flagged). These are pre-merge developer mistakes, not new attacker-controlled sinks; the guard adds defense-in-depth and does not replace authn/authz or remove existing controls. CI does not yet invoke `lint-log-secrets` (only `make lint` locally) — enforcement gap, not a vulnerability introduced by this diff. **Indexer runtime (unchanged by MR):** Startup logging already limits fields to non-secret config (`run_mode`, `lcd_urls`, `factory_address`, rate limits); `database_url` used at `connect()` only. Security review: **no medium+ findings** on this diff. No `block:security` label applied.

Brouie commented

2026-06-29 16:19:21 +00:00

(Migrated from gitlab.com)

mentioned in merge request !964