Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JSONbored merged 2 commits into from
Jul 3, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ac0045
fix(selfhost): fix Grafana reporting exporter's Postgres connection a…
JSONbored Jul 3, 2026
864bc70
fix(selfhost): handle bracketed IPv6 hosts in the Postgres URL splitter
JSONbored Jul 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 63 additions & 6 deletions scripts/export-grafana-reporting-db.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,64 @@ pg_enabled() {
esac
}

# Split $PG_DB (a postgres://[user[:pass]@]host[:port]/dbname[?query] URL) into separate PGHOST/PGPORT/PGUSER/
# PGPASSWORD/PGDATABASE/PGSSLMODE env vars and export them, so every psql call below authenticates purely
# through the environment: never a URL on argv (the credential-leak-via-`ps`-listings concern #2461 addressed)
# and never through PGDATABASE holding the WHOLE url (the #2461 regression -- unlike passing the url as psql's
# positional dbname argument, PGDATABASE is not URI-expanded by libpq; it is taken as a literal, nonexistent
# database name, so psql falls back to a local Unix-socket connection attempt instead of the intended TCP host).
# Known limitation: no percent-decoding of the user/password segment (matches this script's pre-existing scope --
# neither the original positional-arg form nor this one has ever decoded a percent-encoded credential).
pg_export_connection_env() {
rest="${PG_DB#*://}"
no_query="${rest%%\?*}"
case "$rest" in
"$no_query"'?'*) query="${rest#"$no_query"?}" ;;
*) query="" ;;
esac
case "$no_query" in
*@*) userinfo="${no_query%%@*}"; hostpart="${no_query#*@}" ;;
*) userinfo=""; hostpart="$no_query" ;;
esac
case "$hostpart" in
*/*) hostport="${hostpart%%/*}"; PGDATABASE="${hostpart#*/}" ;;
*) hostport="$hostpart"; PGDATABASE="" ;;
esac
# An IPv6 literal is bracketed in URI syntax (RFC 3986) specifically because it contains colons itself --
# e.g. postgres://u:p@[::1]:5432/db -- so a naive split on the FIRST colon wrongly cuts the address apart
# (PGHOST='[', PGPORT=':1]:5432'). Handle the bracketed forms (with and without a trailing port) before
# falling back to plain first-colon splitting for an ordinary hostname/IPv4 host. psql/libpq accept the
# IPv6 literal via PGHOST WITHOUT its brackets (brackets are only a URI-syntax disambiguator).
case "$hostport" in
\[*\]:*)
PGHOST="${hostport#\[}"
PGHOST="${PGHOST%%\]:*}"
PGPORT="${hostport##*\]:}"
;;
\[*\])
PGHOST="${hostport#\[}"
PGHOST="${PGHOST%\]}"
PGPORT=""
;;
*:*) PGHOST="${hostport%%:*}"; PGPORT="${hostport#*:}" ;;
*) PGHOST="$hostport"; PGPORT="" ;;
esac
case "$userinfo" in
*:*) PGUSER="${userinfo%%:*}"; PGPASSWORD="${userinfo#*:}" ;;
*) PGUSER="$userinfo"; PGPASSWORD="" ;;
esac
export PGHOST PGPORT PGUSER PGPASSWORD PGDATABASE
case "$query" in
*sslmode=*)
sslpart="${query#*sslmode=}"
PGSSLMODE="${sslpart%%&*}"
export PGSSLMODE
;;
esac
}

pg_scalar() {
PGDATABASE="$PG_DB" psql -X -q -t -A -v ON_ERROR_STOP=1 -c "$1"
psql -X -q -t -A -v ON_ERROR_STOP=1 -c "$1"
}

pg_table_exists() {
Expand All @@ -63,7 +119,7 @@ pg_column_exists() {
pg_copy_csv() {
query="$1"
out="$2"
PGDATABASE="$PG_DB" psql -X -q -v ON_ERROR_STOP=1 -c "COPY ($query) TO STDOUT WITH CSV" >"$out"
psql -X -q -v ON_ERROR_STOP=1 -c "COPY ($query) TO STDOUT WITH CSV" >"$out"
}

sqlite_import_csv() {
Expand Down Expand Up @@ -119,6 +175,7 @@ if pg_enabled; then
echo "reporting export failed: DATABASE_URL is Postgres but psql is not installed" >&2
exit 1
fi
pg_export_connection_env

if ! pg_table_exists "pull_requests" &&
! pg_table_exists "advisories" &&
Expand Down Expand Up @@ -155,15 +212,15 @@ current_pull_requests AS (
CASE
WHEN lower(p.state) = 'closed' AND p.merged_at IS NOT NULL THEN 'merged'
WHEN lower(p.state) = 'closed' THEN 'closed'
WHEN a.conclusion IN ('failure', 'action_required') THEN 'manual'
WHEN a.conclusion IN ('failure', 'action_required', 'neutral') THEN 'manual'
WHEN a.conclusion IS NOT NULL THEN 'commented'
ELSE 'manual'
END AS status,
CASE a.conclusion
WHEN 'success' THEN 'merge'
WHEN 'failure' THEN 'close'
WHEN 'action_required' THEN 'manual'
WHEN 'neutral' THEN 'comment'
WHEN 'neutral' THEN 'manual'
WHEN 'skipped' THEN 'ignore'
ELSE NULL
END AS verdict,
Expand Down Expand Up @@ -298,15 +355,15 @@ current_pull_requests AS (
CASE
WHEN lower(p.state) = 'closed' AND p.merged_at IS NOT NULL THEN 'merged'
WHEN lower(p.state) = 'closed' THEN 'closed'
WHEN a.conclusion IN ('failure', 'action_required') THEN 'manual'
WHEN a.conclusion IN ('failure', 'action_required', 'neutral') THEN 'manual'
WHEN a.conclusion IS NOT NULL THEN 'commented'
ELSE 'manual'
END AS status,
CASE a.conclusion
WHEN 'success' THEN 'merge'
WHEN 'failure' THEN 'close'
WHEN 'action_required' THEN 'manual'
WHEN 'neutral' THEN 'comment'
WHEN 'neutral' THEN 'manual'
WHEN 'skipped' THEN 'ignore'
ELSE NULL
END AS verdict,
Expand Down
72 changes: 68 additions & 4 deletions test/unit/selfhost-grafana-reporting.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { execFileSync } from "node:child_process";
import { chmodSync, existsSync, mkdirSync, mkdtempSync, readdirSync, rmSync, writeFileSync } from "node:fs";
import { chmodSync, existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, describe, expect, it } from "vitest";
Expand Down Expand Up @@ -56,8 +56,14 @@ case " $args " in
exit 8
;;
esac
if [ "\${PGDATABASE:-}" != "postgres://gittensory:pw@postgres:5432/gittensory" ]; then
echo 'psql did not receive postgres URL through PGDATABASE' >&2
case " \${PGHOST:-} \${PGPORT:-} \${PGUSER:-} \${PGPASSWORD:-} \${PGDATABASE:-} " in
*" postgres://"*|*" postgresql://"*)
echo 'psql received the whole postgres URL through an env var instead of split components' >&2
exit 8
;;
esac
if [ "\${PGHOST:-}" != "postgres" ] || [ "\${PGPORT:-}" != "5432" ] || [ "\${PGUSER:-}" != "gittensory" ] || [ "\${PGPASSWORD:-}" != "pw" ] || [ "\${PGDATABASE:-}" != "gittensory" ]; then
echo 'psql did not receive the split connection env vars (PGHOST/PGPORT/PGUSER/PGPASSWORD/PGDATABASE)' >&2
exit 8
fi
case "$args" in
Expand Down Expand Up @@ -124,6 +130,26 @@ esac
return bin;
}

// Captures the connection env vars psql actually receives (rather than asserting a fixed expected value inline
// like fakePsql) so callers can point it at ANY DATABASE_URL shape and inspect exactly what was parsed out.
function capturingPsql(root: string): { bin: string; captureFile: string } {
const bin = join(root, "capture-bin");
mkdirSync(bin);
const psql = join(bin, "psql");
const captureFile = join(root, "captured-env.txt");
writeFileSync(
psql,
`#!/bin/sh
printf 'PGHOST=%s\\nPGPORT=%s\\nPGUSER=%s\\nPGPASSWORD=%s\\nPGDATABASE=%s\\n' "\${PGHOST:-}" "\${PGPORT:-}" "\${PGUSER:-}" "\${PGPASSWORD:-}" "\${PGDATABASE:-}" > "${captureFile}"
case "$*" in
*"information_schema.tables"*) printf '1\\n' ;;
esac
`,
);
chmodSync(psql, 0o755);
return { bin, captureFile };
}

function failingPsql(root: string): string {
const bin = join(root, "broken-bin");
mkdirSync(bin);
Expand Down Expand Up @@ -219,8 +245,11 @@ esac

expect(sqlite(outDb, "PRAGMA quick_check;")).toBe("ok");
expect(sqlite(outDb, "SELECT count(*) FROM review_targets;")).toBe("3");
// Latest advisory conclusion for #1690 is 'neutral' -- counted as 'manual' (matches gateHeld's held-for-review
// definition in src/signals/engine.ts, not 'commented'/'comment') so the dashboard's manual-review panel
// reflects the same held state the live app itself surfaces via gittensory:needs-human-review.
expect(sqlite(outDb, "SELECT submitter || '|' || status || '|' || verdict || '|' || updated_at FROM review_targets WHERE repo='JSONbored/gittensory' AND number=1690;")).toBe(
"JSONbored|commented|comment|2026-06-28T21:40:00Z",
"JSONbored|manual|manual|2026-06-28T21:40:00Z",
);
expect(sqlite(outDb, "SELECT status || '|' || verdict || '|' || updated_at FROM review_targets WHERE repo='JSONbored/gittensory' AND number=1691;")).toBe(
"merged|merge|2026-06-28T21:47:40Z",
Expand Down Expand Up @@ -347,6 +376,41 @@ esac
expect(sqlite(outDb, "SELECT sum(estimated_neurons) FROM ai_usage_events;")).toBe("42");
});

it("REGRESSION (gate-flagged): a bracketed IPv6 Postgres host (postgres://u:p@[::1]:5432/db) is split correctly, not cut apart at the address's own internal colons", () => {
const root = tmpRoot();
const outDb = join(root, "reporting.sqlite");
const { bin, captureFile } = capturingPsql(root);

runExporter(root, join(root, "unused.sqlite"), outDb, {
DATABASE_URL: "postgres://gittensory:pw@[::1]:5432/gittensory",
PATH: `${bin}:${process.env.PATH ?? ""}`,
});

const captured = readFileSync(captureFile, "utf8");
expect(captured).toContain("PGHOST=::1\n");
expect(captured).toContain("PGPORT=5432\n");
expect(captured).toContain("PGUSER=gittensory\n");
expect(captured).toContain("PGPASSWORD=pw\n");
expect(captured).toContain("PGDATABASE=gittensory\n");
});

it("REGRESSION: a bracketed IPv6 Postgres host with no port and no userinfo (postgres://[::1]/db) is split correctly", () => {
const root = tmpRoot();
const outDb = join(root, "reporting.sqlite");
const { bin, captureFile } = capturingPsql(root);

runExporter(root, join(root, "unused.sqlite"), outDb, {
DATABASE_URL: "postgres://[::1]/gittensory",
PATH: `${bin}:${process.env.PATH ?? ""}`,
});

const captured = readFileSync(captureFile, "utf8");
expect(captured).toContain("PGHOST=::1\n");
expect(captured).toContain("PGPORT=\n");
expect(captured).toContain("PGUSER=\n");
expect(captured).toContain("PGDATABASE=gittensory\n");
});

it("fails closed when Postgres metadata cannot be inspected", () => {
const root = tmpRoot();
const outDb = join(root, "reporting.sqlite");
Expand Down
Loading