Say which part of DATABASE_URL is not percent-encoded, instead of failing as URI error

CHANGELOG.md

@@ -8,6 +8,16 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+### A password with a `%` in it says so, instead of failing as `URI error`
+
+`DATABASE_URL` is taken apart before it reaches Bun, and each part is percent-decoded. A part
+holding a `%` that starts no escape -- `postgres://openbot:100%pure@host:5432/openbot`, which a
+generated password produces often enough -- is a string `new URL` accepts and `decodeURIComponent`
+rejects, so the server stopped with `URIError: URI error` and named neither the variable nor the
+part. It now refuses with the same kind of sentence as every other malformed address: which part is
+wrong, and that a literal `%` must be written `%25`.
+
+A correctly encoded password is unaffected.
 ### An empty Bot `PORT` is unset, so NaN never reaches Bun.serve
 
 `PORT=` on `agent-bot` and `agent-langgraph` used to parse as `NaN` (`??` does not treat empty as absent) and `Bun.serve` bound an ephemeral port while compose still published 4200/4201. A prefix typo (`42o0`) started on 42. Empty now means the shipped default; anything that is not a whole port number refuses to start.

server/src/db/client.ts

@@ -3,11 +3,25 @@ import { drizzle } from "drizzle-orm/bun-sql";
 import * as schema from "./schema";
 
 /**
- * `max` is exposed so tests can pin the pool to a single connection. Code that opens a transaction
- * and then reads on a second connection deadlocks once every pooled connection is inside such a
- * transaction; a pool of one turns that from a load-dependent production hang into an immediate,
- * reproducible failure.
+ * One percent-decoded part of the address, or a refusal that names it.
+ *
+ * A password is where this bites. `postgres://openbot:100%pure@host:5432/openbot` is a string
+ * `new URL` accepts without complaint, and `decodeURIComponent` then rejects with `URIError: URI
+ * error` -- a message that names neither `DATABASE_URL` nor which part of it was wrong, thrown out
+ * of the one function whose whole job is to make a connection failure legible. A `%` that starts no
+ * escape is a common thing to find in a generated password, and every other malformed address here
+ * is answered with a sentence saying what to fix.
  */
+function decodePart(value: string, part: string): string {
+  try {
+    return decodeURIComponent(value);
+  } catch {
+    throw new TypeError(
+      `DATABASE_URL has a ${part} that is not percent-encoded. A literal "%" must be written "%25".`,
+    );
+  }
+}
+
 /**
  * The address, taken apart, because Bun will not take it whole on every platform.
  *
@@ -35,7 +49,7 @@ function addressOf(databaseUrl: string) {
       "DATABASE_URL names no host. Expected postgres://user:password@host:port/database.",
     );
   }
-  const database = decodeURIComponent(url.pathname.replace(/^\//, ""));
+  const database = decodePart(url.pathname.replace(/^\//, ""), "database name");
   if (database === "") {
     throw new TypeError(
       "DATABASE_URL names no database. Expected postgres://user:password@host:port/database.",
@@ -55,13 +69,19 @@ function addressOf(databaseUrl: string) {
     adapter: "postgres" as const,
     hostname: url.hostname,
     port: url.port === "" ? 5432 : Number(url.port),
-    username: decodeURIComponent(url.username),
-    password: decodeURIComponent(url.password),
+    username: decodePart(url.username, "username"),
+    password: decodePart(url.password, "password"),
     database,
     ...(Object.keys(connection).length > 0 ? { connection } : {}),
   };
 }
 
+/**
+ * `max` is exposed so tests can pin the pool to a single connection. Code that opens a transaction
+ * and then reads on a second connection deadlocks once every pooled connection is inside such a
+ * transaction; a pool of one turns that from a load-dependent production hang into an immediate,
+ * reproducible failure.
+ */
 export function createDatabase(
   databaseUrl: string,
   options: { max?: number } = {},

server/tests/db-client-address.test.ts

@@ -46,6 +46,37 @@ describe("the database address", () => {
     ).toThrow(/names no database/);
   });
 
+  test("refuses a password holding a percent that starts no escape, naming the part", () => {
+    /*
+     * `new URL` accepts this and `decodeURIComponent` does not, so the refusal used to be a bare
+     * `URIError: URI error` naming neither DATABASE_URL nor the password -- out of the one function
+     * whose job is to make a connection failure legible. A generated password is a common place to
+     * find a literal `%`.
+     */
+    expect(() =>
+      createDatabase("postgres://openbot:100%pure@127.0.0.1:5432/openbot"),
+    ).toThrow(/DATABASE_URL has a password that is not percent-encoded/);
+  });
+
+  test("refuses a username holding one too", () => {
+    expect(() =>
+      createDatabase("postgres://open%bot:openbot@127.0.0.1:5432/openbot"),
+    ).toThrow(/DATABASE_URL has a username that is not percent-encoded/);
+  });
+
+  test("refuses a database name holding one too", () => {
+    expect(() =>
+      createDatabase("postgres://openbot:openbot@127.0.0.1:5432/open%bot"),
+    ).toThrow(/DATABASE_URL has a database name that is not percent-encoded/);
+  });
+
+  test("still accepts a password that IS percent-encoded, decoding it", () => {
+    // The escape a correctly written password uses: `%40` is `@`, which cannot be written raw.
+    expect(() =>
+      createDatabase("postgres://openbot:p%40ss@127.0.0.1:5432/openbot"),
+    ).not.toThrow();
+  });
+
   test("still refuses pool options where the address belongs", () => {
     // @ts-expect-error the wrong-way-round call this guard exists for
     expect(() => createDatabase({ max: 1 })).toThrow(/connection string/);