diff --git a/scripts/export-grafana-reporting-db.sh b/scripts/export-grafana-reporting-db.sh index 7d0ced17b6..ab19c25704 100644 --- a/scripts/export-grafana-reporting-db.sh +++ b/scripts/export-grafana-reporting-db.sh @@ -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() { @@ -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() { @@ -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" && @@ -155,7 +212,7 @@ 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, @@ -163,7 +220,7 @@ current_pull_requests AS ( 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, @@ -298,7 +355,7 @@ 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, @@ -306,7 +363,7 @@ current_pull_requests AS ( 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, diff --git a/test/unit/selfhost-grafana-reporting.test.ts b/test/unit/selfhost-grafana-reporting.test.ts index 96298d3695..cf2654f1f0 100644 --- a/test/unit/selfhost-grafana-reporting.test.ts +++ b/test/unit/selfhost-grafana-reporting.test.ts @@ -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"; @@ -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 @@ -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); @@ -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", @@ -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");