Skip to content

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

Merged
davidmckayv merged 3 commits into
CopilotKit:mainfrom
kevin9327:fix/database-url-percent
Sep 6, 2026
Merged

Say which part of DATABASE_URL is not percent-encoded, instead of failing as URI error#400
davidmckayv merged 3 commits into
CopilotKit:mainfrom
kevin9327:fix/database-url-percent

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

What this changes

A DATABASE_URL whose password holds a % that starts no escape stops the server with

URIError: URI error

which names neither the variable nor which part of it was wrong. It comes out of addressOf, the
one function whose entire job is to make a connection failure legible, and which answers every other
malformed address with a sentence saying what to fix:

DATABASE_URL is not a URL: "://openbot@/openbot"
DATABASE_URL names no host. Expected postgres://user:password@host:port/database.
DATABASE_URL names no database. Expected postgres://user:password@host:port/database.

new URL accepts the string, so the failure lands two lines later on decodeURIComponent, which
was added in #384 alongside taking the address apart. Measured, on Bun 1.3.14:

postgres://openbot:100%pure@127.0.0.1:5432/openbot   URIError: URI error
postgres://openbot:pa%ss@127.0.0.1:5432/openbot      URIError: URI error
postgres://open%bot:openbot@127.0.0.1:5432/openbot   URIError: URI error
postgres://openbot:openbot@127.0.0.1:5432/open%bot   URIError: URI error
postgres://openbot:p%40ss@127.0.0.1:5432/openbot     connects, password is p@ss

A generated password is a common place to find a literal %, and this is a startup failure with no
retry and nothing in the message to search for.

decodePart wraps the three decodes and refuses with the same shape as its neighbours: which part,
and that a literal % must be written %25. A correctly encoded password decodes exactly as it did.

I also moved the max doc comment down onto createDatabase, where it belongs. It was already
detached — it sat above addressOf's own doc block — and adding a function there would have made it
read as documentation for the new one.

Where it runs

  • New state that outlives a request? None. A pure function over a string, called once at
    startup.
  • What happens on the second replica? The same as on the first: it refuses the same address
    with the same sentence. Every replica reads the same DATABASE_URL, so this is a change of
    message, not of behaviour that can differ per process.
  • Anything serialised? No.
  • Anything fanned out to a browser? No. This is reached before the server listens.
  • New listener, port, or schedule? None.

Boundary and audit

  • Every acting call still goes through the gateway: no request path is touched.
  • New refusals and new failures each write a row: not applicable. This refusal happens before
    the database exists, so there is nowhere to write a row to and nothing has yet acted.
  • Nothing new is trusted from the client: the input is an operator's environment variable.

Changelog

  • A line in CHANGELOG.md under Unreleased.

Tests

Four in server/tests/db-client-address.test.ts, alongside the refusals already enumerated there:

  • a password holding a bare % is refused, naming the password
  • a username holding one is refused, naming the username
  • a database name holding one is refused, naming the database name
  • p%40ss still decodes to p@ss, so a correctly written password is untouched

Against main the first three fail; the fourth passes before and after and is there as the
regression guard.

How I tested

Windows 11, Bun 1.3.14. bun test server/tests/db-client-address.test.ts is 9 passed, 1 failed —
the failure is connection parameters on the URL > survive…, which opens a real socket to Postgres
and fails identically on main on this machine, where no database is running.
bun run --filter server typecheck and bunx biome check are clean.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@davidmckayv davidmckayv left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep-reviewed against live code (correctness, governance, no vendor/secret/scale issues). Composed build+tests green. CHANGELOG/format rebase on CI-validated substance.

@davidmckayv
davidmckayv merged commit eb1e503 into CopilotKit:main Sep 6, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants