Skip to content

fix(selfhost): fix Grafana reporting exporter's Postgres connection and manual-review classification - #2740

Merged
JSONbored merged 2 commits into
mainfrom
fix/selfhost-grafana-exporter-pg-url
Jul 3, 2026
Merged

fix(selfhost): fix Grafana reporting exporter's Postgres connection and manual-review classification#2740
JSONbored merged 2 commits into
mainfrom
fix/selfhost-grafana-exporter-pg-url

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • `PGDATABASE="$PG_DB" psql` (introduced in fix(selfhost): avoid postgres url argv leaks #2461 to stop leaking the Postgres URL via `ps` listings) treats `PGDATABASE` as a literal database name, not a connection URL -- libpq does not URI-expand it the way it does a positional `dbname` argument. Every Postgres-backed reporting export was silently attempting a local Unix-socket connection and failing. Verified empirically against real psql 17.10.
  • Fix: split the `postgres://` URL into `PGHOST`/`PGPORT`/`PGUSER`/`PGPASSWORD`/`PGDATABASE`/`PGSSLMODE` and export them, so `psql` connects over the intended TCP host while the credential never appears on argv (preserving fix(selfhost): avoid postgres url argv leaks #2461's leak protection).
  • Also broadens the manual-review classification: a `neutral` gate conclusion is now counted as `manual` (matching `gateHeld`'s held-for-review definition in `src/signals/engine.ts`), not silently bucketed as `commented`/`comment` -- so the Grafana "Manual review" panel reflects the same held state the live app surfaces.

Scope

  • Conventional Commit title.
  • Focused, single-script change (+ its test).
  • Follows `CONTRIBUTING.md`.
  • Small enough that no linked issue is needed -- a scoped correctness fix to self-host observability tooling.

Validation

  • `git diff --check`
  • `sh -n scripts/export-grafana-reporting-db.sh`
  • URL parser stress-tested directly against 6 realistic connection-string shapes (with/without password, with/without port, with query string + sslmode)
  • `npx vitest run test/unit/selfhost-grafana-reporting.test.ts` -- 8/8, including the real fake-psql harness executing the actual script end-to-end and asserting the credential never appears on argv
  • `npm audit --audit-level=moderate`
  • scripts/** and test/** are Codecov-exempt; correctness verified via the functional shell-script test harness instead

Safety

  • No secrets/credentials committed. The connection URL is still sourced from the operator's own `DATABASE_URL`/`GITTENSORY_REPORTING_SOURCE_DATABASE_URL` env var.
  • N/A: no UI, API, or auth change.

Notes

  • Backend/script-only; no UI Evidence needed.

…nd manual-review classification

PGDATABASE="$PG_DB" psql treats PGDATABASE as a literal database name,
not a connection URL -- libpq does not URI-expand it the way it does a
positional dbname argument, so every Postgres-backed reporting export
was silently falling back to a local Unix-socket connection attempt
and failing. Fix: split the postgres:// URL into PGHOST/PGPORT/PGUSER/
PGPASSWORD/PGDATABASE/PGSSLMODE and export them, so psql connects over
the intended TCP host while the credential never appears on argv
(preserving the process-listing leak protection from #2461).

Also broadens the manual-review classification: a 'neutral' gate
conclusion is now counted as 'manual' (matching gateHeld's held-for-
review definition), not silently bucketed as 'commented'/'comment'.
@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.03%. Comparing base (3ca27bb) to head (864bc70).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2740   +/-   ##
=======================================
  Coverage   96.03%   96.03%           
=======================================
  Files         245      245           
  Lines       27340    27340           
  Branches     9930     9930           
=======================================
  Hits        26255    26255           
  Misses        460      460           
  Partials      625      625           
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb

loopover-orb Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-03 09:58:31 UTC

2 files · 1 AI reviewer · no blockers · readiness 93/100 · CI green · clean

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
The change correctly moves Postgres exporter authentication back to libpq environment variables instead of placing the full URL in PGDATABASE, and it updates neutral advisory conclusions to manual in both SQLite and Postgres export queries. The visible runtime path is coherent, including bracketed IPv6 handling and argv secret avoidance. The main remaining issue is test fidelity: the Postgres fixture still fabricates the old commented/comment row, so it does not verify the changed Postgres classification path.

Nits — 5 non-blocking
  • nit: test/unit/selfhost-grafana-reporting.test.ts:318 still hardcodes the Postgres fake psql result for PR fix(export): redact cached Orb token envelopes #1690 as commented/comment, so the Postgres-side neutral-to-manual SQL change in scripts/export-grafana-reporting-db.sh:212 is not actually exercised by that test.
  • nit: scripts/export-grafana-reporting-db.sh:52 documents percent-decoding only for user/password, but PGDATABASE and sslmode query values are also copied raw from the URI, so the comment should either broaden the limitation or explain the intentionally supported URL subset.
  • Update fakePsql in test/unit/selfhost-grafana-reporting.test.ts:75 to return a neutral advisory-shaped result or assert the generated SQL contains the neutral manual mapping, then change the Postgres export expectation at test/unit/selfhost-grafana-reporting.test.ts:318 to manual|manual.
  • Add one focused parser test for postgresql:​// as well as postgres:​// in test/unit/selfhost-grafana-reporting.test.ts, since pg_enabled accepts both schemes and pg_export_connection_env strips either scheme generically.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 61 registered-repo PR(s), 52 merged, 478 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 61 PR(s), 478 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 61 PR(s), 478 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@loopover-orb loopover-orb Bot added gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 3, 2026
postgres://u:p@[::1]:5432/db split host:port on the first colon, which
cuts a bracketed IPv6 literal apart (PGHOST='[', PGPORT=':1]:5432')
since the address itself contains colons. Handle the bracketed forms
(with and without a trailing port) before falling back to plain
first-colon splitting for an ordinary hostname/IPv4 host.
@JSONbored
JSONbored merged commit e659404 into main Jul 3, 2026
13 checks passed
@JSONbored
JSONbored deleted the fix/selfhost-grafana-exporter-pg-url branch July 3, 2026 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Development

Successfully merging this pull request may close these issues.

1 participant