Skip to content

fix: enforce safety invariants mechanically and cut self-noise - #20

Open
machado144 wants to merge 2 commits into
mainfrom
fix/safety-hardening-and-noise
Open

fix: enforce safety invariants mechanically and cut self-noise#20
machado144 wants to merge 2 commits into
mainfrom
fix/safety-hardening-and-noise

Conversation

@machado144

Copy link
Copy Markdown
Contributor

What

Hardening + honesty fixes that came out of a full audit (checks catalog, safety invariants, reliability, UX) plus live dogfooding against docker-compose MySQL 8.0 / PostgreSQL 15 / Neo4j 5.

Safety / reliability

  • MySQL DSN parameter injection: connections are now built via mysql.Config + NewConnector instead of fmt.Sprintf — a password containing &/@// could previously corrupt the DSN or inject params (worst case: &transaction_read_only=0 disabling read-only hardening). TDD: internal/engine/mysql/mysql_test.go.
  • MySQL lock-wait bounds: sessions now set lock_wait_timeout=5 + innodb_lock_wait_timeout=5 (PG already had lock_timeout=500ms; MySQL had nothing — server default is 1 year).
  • Sampling interval floor: --interval 1ms used to be honored; sampler.New now clamps to 1s so a typo can't hammer production.
  • Invariant feat(engine): adapter core + hardened MySQL/PostgreSQL sessions #1 is now a test: internal/check/safety_test.go walks every registered check at 8 engine/version combos and fails if any FROM/JOIN target is not a system catalog.
  • Timeouts are now proven, not assumed: new integration tests assert statement_timeout/lock_timeout/max_execution_time are in effect, including a pg_sleep(8) that must be server-cancelled at ~3s.
  • CI now runs unit tests: go test -race -count=1 ./... job added — the ~130 unit tests previously never gated a PR.

Insight quality

  • Self-noise fix (found by dogfooding): gumshoe flagged its own diagnostic queries in mysql.statements_full_scan / mysql.top_statements / pg.top_statements. Statement digests referencing only system catalogs are now skipped. The sys view middle-truncates query text, so statements_full_scan joins back to events_statements_summary_by_digest for full text (also improves the displayed samples).
  • pg.top_statements version-gated to PG ≥ 13 — on older servers it errored instead of degrading.

UX

  • Ad-hoc check/watch now prompt for a password on a TTY (README promised this; it never fired — silent auth failure instead).
  • serve warns when bound to a non-loopback address (the web UI is unauthenticated).
  • Filled the empty LICENSE file (MIT — README already claimed it); added go install snippet; synced CLAUDE.md's Neo4j hardening description with the actual implementation.

How to run & test locally

# 1. Start the three test databases
docker compose up -d          # or: make dev-up

# 2. Unit tests + lint + build
make ci

# 3. Integration tests (real DBs, includes the new hardening assertions)
make test-integration         # or: cd integration && go test -tags integration ./...

# 4. Try it live
go run ./cmd/gumshoe check --engine postgres --host 127.0.0.1 --user gumshoe --database testdb   # GUMSHOE_PASSWORD=gumshoe
go run ./cmd/gumshoe serve    # web UI at http://127.0.0.1:8086, connect via the form

# Optional: create a lock pileup and watch gumshoe attribute it
docker exec -d gumshoe-postgres-1 psql -U gumshoe -d testdb -c "BEGIN; UPDATE orders SET total=0 WHERE id=1; SELECT pg_sleep(600);"
docker exec -d gumshoe-postgres-1 psql -U gumshoe -d testdb -c "UPDATE orders SET total=1 WHERE id=1;"
go run ./cmd/gumshoe check --engine postgres --host 127.0.0.1 --user gumshoe --database testdb --threshold long_query_secs=5

Follow-ups (not in this PR)

  • MySQL deadlock + metadata-lock (MDL) wait checks — biggest coverage gap for MySQL
  • PG replication-slot lag / inactive-slot check (the txid_wraparound advice already tells users to look there)
  • Waiter-side lock findings could inline the blocker's user/query (today: blocker-centric finding + per-row "blocked by pid" in Activity)
  • Neo4j check depth (page-cache hit ratio, GC pressure, store size)
  • Homebrew tap + README screenshots/GIF

🤖 Generated with Claude Code

- Build MySQL connections via mysql.Config/NewConnector — a password
  containing DSN metacharacters can no longer corrupt the address or
  inject parameters (e.g. disabling read-only hardening)
- Bound MySQL MDL/InnoDB lock waits (lock_wait_timeout=5,
  innodb_lock_wait_timeout=5), matching PostgreSQL's lock_timeout
- Clamp the sampling interval to a 1s floor so --interval typos cannot
  hammer the target server
- Stop flagging gumshoe's own catalog queries in statement checks
  (mysql.statements_full_scan, mysql.top_statements, pg.top_statements);
  join sys view back to the digest table for full query text
- Add a guard test asserting every registered check reads only system
  catalogs, at every supported engine version
- Add integration tests proving session timeouts are actually in effect
- Gate pg.top_statements on PG >= 13 (column rename would error, not degrade)
- Prompt for a password on ad-hoc check/watch (README promised it)
- Warn when serve binds a non-loopback address (UI is unauthenticated)
- Run unit tests with -race in CI (they previously never gated PRs)
- Fill the empty LICENSE file (MIT); sync CLAUDE.md Neo4j hardening note

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

✅ PR title follows the required format

Current title: fix: enforce safety invariants mechanically and cut self-noise

The marketing site (web/index.html + assets) landed without updating
.structlint.yaml, so every PR since has failed the gauntlet gate on
'Directory not in allowed list: web'. Allow the directory and *.html.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant