diff --git a/scripts/backup.sh b/scripts/backup.sh index 3e80211830..568f40dc22 100644 --- a/scripts/backup.sh +++ b/scripts/backup.sh @@ -10,8 +10,162 @@ RETAIN=${BACKUP_RETAIN:-7} DB=${DATABASE_PATH:-/data/gittensory.sqlite} PG_DB="${GITTENSORY_BACKUP_SOURCE_DATABASE_URL:-${DATABASE_URL:-}}" OUT=${BACKUP_OUT_DIR:-/backups} +PGPASSFILE_CREATED="" +cleanup() { + if [ -n "$PGPASSFILE_CREATED" ]; then + rm -f "$PGPASSFILE_CREATED" + fi +} +trap cleanup EXIT HUP INT TERM mkdir -p "$OUT/postgres" "$OUT/sqlite" "$OUT/qdrant" +# Percent-decodes a URI userinfo component (RFC 3986). Deliberately does NOT treat '+' as a space -- that +# convention is specific to application/x-www-form-urlencoded query values, not URI userinfo, where '+' is +# an ordinary sub-delims character allowed unencoded; the only caller of this function decodes a password +# extracted from the userinfo section, and a literal '+' there must stay a '+', not become a space. +url_decode() { + printf '%s' "$1" | awk ' + BEGIN { for (i = 0; i < 256; i++) hex[sprintf("%02X", i)] = sprintf("%c", i); } + { + out = ""; + for (i = 1; i <= length($0); i++) { + c = substr($0, i, 1); + if (c == "%" && i + 2 <= length($0)) { + h = toupper(substr($0, i + 1, 2)); + if (h in hex) { out = out hex[h]; i += 2; } else { out = out c; } + } else { + out = out c; + } + } + printf "%s", out; + }' +} + +pgpass_escape() { + printf '%s' "$1" | sed 's/\\/\\\\/g; s/:/\\:/g' +} + +# Strips the password from a postgres(ql):// URI -- from EITHER the userinfo (user:password@host) or a +# `password=` libpq query-string parameter (postgresql://user@host/db?password=secret is equally valid +# and equally a leak if left in place) -- and hands pg_dump everything else untouched (host, port, dbname, +# and every other query parameter) as its connection argument, instead of re-parsing those pieces +# ourselves. libpq's own URI parser already handles every form it needs to (query-string-only host, as in +# `postgresql:///db?host=/var/run/postgresql`, IPv6 literals, multi-host strings, sslmode, etc.) -- an +# earlier version of this function extracted host/port/dbname manually and discarded the query string +# entirely, silently breaking any URL that relied on it. Sets $PG_SANITIZED_URL (the password-free URI to +# pass to pg_dump) and, if a password was present, a $PGPASSFILE_CREATED wildcard passfile (host/port/ +# dbname/user are wildcarded: this file exists only to supply the ONE password for the single connection +# this script makes, and is deleted immediately after via the `cleanup` trap, so there's no scoped value +# in re-deriving the exact host/port/dbname libpq will resolve -- which the query string can override +# anyway -- just to match them precisely). +prepare_pg_env() { + pg_rest=${PG_DB#postgres://} + pg_rest=${pg_rest#postgresql://} + + # Userinfo (user:password@) can ONLY appear in the authority component -- everything before the first + # '/', '?', or '#' -- never later in the URI. Find that boundary FIRST and never look for '@'/':' past + # it, or a literal '@'/':' inside a query-string value (e.g. ?application_name=a:b@worker) gets + # misread as credentials, corrupting an otherwise-untouched query string. POSIX parameter expansion has + # no single "find the first of several delimiters" primitive: compute the substring before each + # candidate delimiter and keep whichever is shortest, since the delimiter that occurs earliest produces + # the shortest "before" substring. + pg_authority=${pg_rest%%/*} + pg_before_query=${pg_rest%%\?*} + pg_before_frag=${pg_rest%%#*} + if [ ${#pg_before_query} -lt ${#pg_authority} ]; then pg_authority=$pg_before_query; fi + if [ ${#pg_before_frag} -lt ${#pg_authority} ]; then pg_authority=$pg_before_frag; fi + pg_suffix=${pg_rest#"$pg_authority"} + + PGPASSWORD_VALUE="" + pg_sanitized_authority=$pg_authority + case "$pg_authority" in + *@*) + pg_userinfo=${pg_authority%%@*} + pg_after_at=${pg_authority#*@} + case "$pg_userinfo" in + *:*) + pg_user_part=${pg_userinfo%%:*} + PGPASSWORD_VALUE=$(url_decode "${pg_userinfo#*:}") + pg_sanitized_authority="${pg_user_part}@${pg_after_at}" + ;; + *) + pg_sanitized_authority="${pg_userinfo}@${pg_after_at}" + ;; + esac + ;; + esac + + # A libpq query string can carry `password=...` as an alternative to userinfo -- split $pg_suffix into + # its path / query / fragment components (in that order; each optional) and, if the query component has + # a `password` key, extract and remove it, leaving every other parameter (and their order) untouched. + pg_path=$pg_suffix + pg_query="" + pg_frag="" + case "$pg_suffix" in + *\?*) + pg_path=${pg_suffix%%\?*} + pg_after_q=${pg_suffix#*\?} + case "$pg_after_q" in + *#*) + pg_query=${pg_after_q%%#*} + pg_frag="#${pg_after_q#*#}" + ;; + *) + pg_query=$pg_after_q + ;; + esac + ;; + *#*) + pg_path=${pg_suffix%%#*} + pg_frag="#${pg_suffix#*#}" + ;; + esac + + # A malformed (but not rejected by libpq's own parser) URL could repeat `password=` -- loop until none + # remain rather than stripping only the first, so a leftover second occurrence can never survive into + # $PG_SANITIZED_URL. Each iteration overwrites PGPASSWORD_VALUE, so the LAST occurrence wins; which one + # libpq itself would actually authenticate with is unspecified for a duplicate key, but every occurrence + # is a credential either way and none may reach argv. + pg_query_wrapped="&$pg_query&" + while case "$pg_query_wrapped" in *"&password="*"&"*) true ;; *) false ;; esac; do + pg_before_pw=${pg_query_wrapped%%&password=*} + pg_from_pw=${pg_query_wrapped#*&password=} + PGPASSWORD_VALUE=$(url_decode "${pg_from_pw%%&*}") + pg_after_pw=${pg_from_pw#*&} + pg_query_wrapped="${pg_before_pw}&${pg_after_pw}" + done + pg_query=${pg_query_wrapped#&} + pg_query=${pg_query%&} + + pg_suffix=$pg_path + if [ -n "$pg_query" ]; then pg_suffix="$pg_suffix?$pg_query"; fi + pg_suffix="$pg_suffix$pg_frag" + PG_SANITIZED_URL="postgresql://$pg_sanitized_authority$pg_suffix" + + if [ -n "$PGPASSWORD_VALUE" ]; then + # pgpass is a single-line-per-entry format; pgpass_escape only handles the two characters (':' and + # '\') that format itself treats specially. A decoded password containing a raw newline or carriage + # return would still split the entry across lines, corrupting the field layout -- refuse outright + # rather than silently write a malformed passfile. + # + # NOTE: "$(printf '\n')" as a case pattern would NOT work here -- command substitution strips ALL + # trailing newlines, so it evaluates to an empty string and the pattern would match everything. Build + # a variable holding exactly one newline/CR by stripping a trailing marker byte instead. + pg_nl=$(printf '\nx'); pg_nl=${pg_nl%x} + pg_cr=$(printf '\rx'); pg_cr=${pg_cr%x} + case "$PGPASSWORD_VALUE" in + *"$pg_nl"*|*"$pg_cr"*) + echo "[backup] refusing to write PGPASSFILE: decoded Postgres password contains a newline or carriage return" >&2 + exit 1 + ;; + esac + PGPASSFILE_CREATED=$(mktemp "${TMPDIR:-/tmp}/gittensory-pgpass.XXXXXX") + chmod 600 "$PGPASSFILE_CREATED" + printf '*:*:*:*:%s\n' "$(pgpass_escape "$PGPASSWORD_VALUE")" > "$PGPASSFILE_CREATED" + export PGPASSFILE="$PGPASSFILE_CREATED" + fi +} + # 1) Active app database. Prefer Postgres when DATABASE_URL is set; otherwise keep the SQLite online backup path. case "$PG_DB" in postgres://*|postgresql://*) @@ -19,7 +173,8 @@ case "$PG_DB" in echo "[backup] pg_dump not found; cannot back up Postgres database" >&2 exit 1 fi - pg_dump -Fc -f "$OUT/postgres/gittensory-$TS.dump" "$PG_DB" + prepare_pg_env + pg_dump -Fc -f "$OUT/postgres/gittensory-$TS.dump" "$PG_SANITIZED_URL" echo "[backup] postgres -> $OUT/postgres/gittensory-$TS.dump" ;; *) diff --git a/test/unit/selfhost-backup-script.test.ts b/test/unit/selfhost-backup-script.test.ts index c9cfd60328..84fbd109cd 100644 --- a/test/unit/selfhost-backup-script.test.ts +++ b/test/unit/selfhost-backup-script.test.ts @@ -28,6 +28,7 @@ function fakePgDump(root: string): string { join(bin, "pg_dump"), `#!/bin/sh out='' +original_args="$*" while [ "$#" -gt 0 ]; do case "$1" in -f) @@ -43,6 +44,22 @@ if [ -z "$out" ]; then echo 'missing -f output' >&2 exit 2 fi +if [ -n "\${PG_DUMP_ARGS_FILE:-}" ]; then + printf '%s\\n' "$original_args" > "$PG_DUMP_ARGS_FILE" +fi +if [ -n "\${PG_DUMP_ENV_FILE:-}" ]; then + passfile_path="\${PGPASSFILE:-}" + passfile_content="" + passfile_mode_ok="no" + if [ -n "$passfile_path" ] && [ -f "$passfile_path" ]; then + passfile_content="$(cat "$passfile_path")" + # find -perm is portable across BSD (macOS) and GNU (Linux) find, unlike \`stat\`'s incompatible flags. + case "$(find "$passfile_path" -perm 600 2>/dev/null)" in + "$passfile_path") passfile_mode_ok="yes" ;; + esac + fi + printf '%s|%s|%s\\n' "$passfile_path" "$passfile_content" "$passfile_mode_ok" > "$PG_DUMP_ENV_FILE" +fi printf 'postgres dump\\n' > "$out" `, ); @@ -91,6 +108,8 @@ describe("self-host backup script", () => { const output = runBackup(root, { DATABASE_URL: "postgres://gittensory:pw@postgres:5432/gittensory", + PG_DUMP_ARGS_FILE: join(root, "pg-dump.args"), + PG_DUMP_ENV_FILE: join(root, "pg-dump.env"), DATABASE_PATH: staleSqlite, PATH: `${pgBin}:${process.env.PATH ?? ""}`, }); @@ -102,6 +121,193 @@ describe("self-host backup script", () => { expect(readdirSync(join(root, "backups", "sqlite"))).toEqual([]); }); + it("does not pass the Postgres password in pg_dump arguments, but keeps host/port/dbname/user reachable via a sanitized URL", () => { + const root = tmpRoot(); + const pgBin = fakePgDump(root); + const argsFile = join(root, "pg-dump.args"); + const envFile = join(root, "pg-dump.env"); + + runBackup(root, { + DATABASE_URL: "postgresql://app_user:SuperSecret123%21@db.example:6543/gittensory", + PATH: `${pgBin}:${process.env.PATH ?? ""}`, + PG_DUMP_ARGS_FILE: argsFile, + PG_DUMP_ENV_FILE: envFile, + }); + + const args = execFileSync("cat", [argsFile], { encoding: "utf8" }); + const pgEnv = execFileSync("cat", [envFile], { encoding: "utf8" }).trim(); + const [passfilePath, passfileContent, passfileModeOk] = pgEnv.split("|"); + + // The password (percent-encoded or decoded) must never appear on argv. + expect(args).not.toContain("SuperSecret123"); + expect(args).not.toContain("app_user:"); + // Host/port/dbname/user are pg_dump's connection info, not secrets -- libpq resolves them from this + // sanitized (password-free) URL exactly as it would have from the original. + expect(args).toContain("postgresql://app_user@db.example:6543/gittensory"); + // The password reaches pg_dump out-of-band via a 600-permission PGPASSFILE, url-decoded. Match on the + // basename only, not a hardcoded /tmp/ prefix: mktemp resolves under $TMPDIR, which macOS sets to a + // per-user private directory rather than /tmp (the CI runner's Linux environment does default to /tmp, + // but the assertion shouldn't assume that). + expect(passfilePath).toMatch(/\/gittensory-pgpass\.[^/]+$/); + expect(passfileContent).toBe("*:*:*:*:SuperSecret123!"); + expect(passfileModeOk).toBe("yes"); + }); + + it("preserves query-string-only connection info that a host/port/dbname split would otherwise drop", () => { + const root = tmpRoot(); + const pgBin = fakePgDump(root); + const argsFile = join(root, "pg-dump.args"); + + // No authority host at all -- the actual connection target is supplied entirely via the query string + // (a valid, real-world libpq URI form for connecting over a Unix socket at a non-default path). + runBackup(root, { + DATABASE_URL: "postgresql:///gittensory?host=/var/run/postgresql", + PATH: `${pgBin}:${process.env.PATH ?? ""}`, + PG_DUMP_ARGS_FILE: argsFile, + }); + + const args = execFileSync("cat", [argsFile], { encoding: "utf8" }); + expect(args).toContain("postgresql:///gittensory?host=/var/run/postgresql"); + }); + + it("strips a password supplied via the libpq query-string form, not just userinfo", () => { + const root = tmpRoot(); + const pgBin = fakePgDump(root); + const argsFile = join(root, "pg-dump.args"); + const envFile = join(root, "pg-dump.env"); + + // postgresql://user@host/db?password=... is an equally valid, if less common, way to supply a libpq + // password -- entirely independent of the userinfo form this function already strips. Includes other + // query parameters on both sides of `password` to prove they survive, in order, untouched. + runBackup(root, { + DATABASE_URL: "postgresql://app_user@db.example:6543/gittensory?sslmode=require&password=SuperSecret123%21&application_name=app", + PATH: `${pgBin}:${process.env.PATH ?? ""}`, + PG_DUMP_ARGS_FILE: argsFile, + PG_DUMP_ENV_FILE: envFile, + }); + + const args = execFileSync("cat", [argsFile], { encoding: "utf8" }); + const [, passfileContent] = execFileSync("cat", [envFile], { encoding: "utf8" }).trim().split("|"); + expect(args).not.toContain("SuperSecret123"); + expect(args).not.toContain("password="); + expect(args).toContain("postgresql://app_user@db.example:6543/gittensory?sslmode=require&application_name=app"); + expect(passfileContent).toBe("*:*:*:*:SuperSecret123!"); + }); + + it("strips EVERY occurrence of a repeated query-string password, not just the first", () => { + const root = tmpRoot(); + const pgBin = fakePgDump(root); + const argsFile = join(root, "pg-dump.args"); + + // A malformed URL repeating `password=` isn't rejected by libpq's own parser -- stripping only the + // first occurrence would leave a second one sitting in argv, still a leaked credential regardless of + // which one libpq itself would actually authenticate with. + runBackup(root, { + DATABASE_URL: "postgresql://u@h/db?password=oneSecret&sslmode=require&password=twoSecret", + PATH: `${pgBin}:${process.env.PATH ?? ""}`, + PG_DUMP_ARGS_FILE: argsFile, + }); + + const args = execFileSync("cat", [argsFile], { encoding: "utf8" }); + expect(args).not.toContain("oneSecret"); + expect(args).not.toContain("twoSecret"); + expect(args).not.toContain("password="); + expect(args).toContain("postgresql://u@h/db?sslmode=require"); + }); + + it("does not mistake a query value merely containing the substring 'password' for the password key", () => { + const root = tmpRoot(); + const pgBin = fakePgDump(root); + const argsFile = join(root, "pg-dump.args"); + + const url = "postgresql://host/db?application_name=has_password_in_name&other=1"; + runBackup(root, { + DATABASE_URL: url, + PATH: `${pgBin}:${process.env.PATH ?? ""}`, + PG_DUMP_ARGS_FILE: argsFile, + }); + + const args = execFileSync("cat", [argsFile], { encoding: "utf8" }); + expect(args).toContain(url); + }); + + it("does not mistake a literal '@'/':' inside the query string for userinfo", () => { + const root = tmpRoot(); + const pgBin = fakePgDump(root); + const argsFile = join(root, "pg-dump.args"); + const envFile = join(root, "pg-dump.env"); + + // No userinfo here at all -- the '@' and ':' both belong to a query parameter VALUE. Userinfo can + // only appear in the authority (before the first '/', '?', or '#'); scanning the whole remaining + // string for the first '@' would wrongly treat "gittensory?application_name=a:b" as userinfo and + // strip ":b" out as a fake password, corrupting the URL passed to pg_dump. + const url = "postgresql://db.example/gittensory?application_name=a:b@worker"; + runBackup(root, { + DATABASE_URL: url, + PATH: `${pgBin}:${process.env.PATH ?? ""}`, + PG_DUMP_ARGS_FILE: argsFile, + PG_DUMP_ENV_FILE: envFile, + }); + + const args = execFileSync("cat", [argsFile], { encoding: "utf8" }); + const [passfilePath] = execFileSync("cat", [envFile], { encoding: "utf8" }).trim().split("|"); + expect(args).toContain(url); + expect(passfilePath).toBe(""); + }); + + it("extracts a real password even when the query string separately contains '@'/':'", () => { + const root = tmpRoot(); + const pgBin = fakePgDump(root); + const argsFile = join(root, "pg-dump.args"); + const envFile = join(root, "pg-dump.env"); + + runBackup(root, { + DATABASE_URL: "postgres://user:realpass@host/db?application_name=a:b@worker", + PATH: `${pgBin}:${process.env.PATH ?? ""}`, + PG_DUMP_ARGS_FILE: argsFile, + PG_DUMP_ENV_FILE: envFile, + }); + + const args = execFileSync("cat", [argsFile], { encoding: "utf8" }); + const [, passfileContent] = execFileSync("cat", [envFile], { encoding: "utf8" }).trim().split("|"); + expect(args).not.toContain("realpass"); + expect(args).toContain("postgresql://user@host/db?application_name=a:b@worker"); + expect(passfileContent).toBe("*:*:*:*:realpass"); + }); + + it("keeps a literal '+' in the password as '+', not a decoded space", () => { + const root = tmpRoot(); + const pgBin = fakePgDump(root); + const envFile = join(root, "pg-dump.env"); + + // '+' means "space" only in application/x-www-form-urlencoded query values, not in a URI's userinfo + // component, where it's an ordinary allowed character -- decoding it as a space would corrupt any + // password containing one. + runBackup(root, { + DATABASE_URL: "postgres://user:pass+word@host/db", + PATH: `${pgBin}:${process.env.PATH ?? ""}`, + PG_DUMP_ENV_FILE: envFile, + }); + + const [, passfileContent] = execFileSync("cat", [envFile], { encoding: "utf8" }).trim().split("|"); + expect(passfileContent).toBe("*:*:*:*:pass+word"); + }); + + it("refuses to write a PGPASSFILE when the decoded password contains a newline", () => { + const root = tmpRoot(); + const pgBin = fakePgDump(root); + + // pgpass_escape only escapes ':' and '\' -- a raw newline (here percent-encoded as %0A) would still + // split the entry across lines and corrupt the single-line pgpass format, so this must fail loudly + // rather than silently write a malformed passfile. + expect(() => + runBackup(root, { + DATABASE_URL: "postgres://user:bad%0Apassword@host/db", + PATH: `${pgBin}:${process.env.PATH ?? ""}`, + }), + ).toThrow(/backup\.sh/); + }); + it("keeps the SQLite online backup path when no Postgres URL is configured", () => { const root = tmpRoot(); const sqliteBin = fakeSqlite(root);