mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-10 16:05:58 +00:00
fix(frontend): read connection parameters the way libpq does
One reader for both the parser and the allowlist, since they disagreed about what a string says in two ways that both ended in a weaker connection than was pasted: - `URLSearchParams.get` takes the first of a repeated parameter and libpq takes the last, so `?sslmode=disable&sslmode=require` was read as `disable`. - The allowlist folded the parameter name and the parser did not, so `?SslMode=verify-full` was refused by neither and honoured by neither, and saved as the `require` default. The parked Supabase run is now handed to `open()` rather than read back off the `resume` prop it was just assigned to, so restoring it does not depend on when that prop reaches the component. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
98fd2d8873
commit
fcbcf221f5
@@ -353,13 +353,13 @@
|
||||
})
|
||||
})
|
||||
|
||||
function reset() {
|
||||
function reset(from: WizardResume | undefined) {
|
||||
wiz = newWizardState({
|
||||
name: resume?.name || defaultTableName(),
|
||||
projectName: resume?.projectName || defaultProjectName(),
|
||||
name: from?.name || defaultTableName(),
|
||||
projectName: from?.projectName || defaultProjectName(),
|
||||
folder: defaultFolder()
|
||||
})
|
||||
wiz.supabase.region = resume?.region ?? DEFAULT_SUPABASE_REGION
|
||||
wiz.supabase.region = from?.region ?? DEFAULT_SUPABASE_REGION
|
||||
run = { steps: [], running: false }
|
||||
maxStep = 1
|
||||
// What the last run claimed belongs to the data table it created; a fresh one has to
|
||||
@@ -372,38 +372,39 @@
|
||||
lastFailure = ''
|
||||
pathTakenError = ''
|
||||
poolerUnavailable = undefined
|
||||
if (resume) {
|
||||
if (from) {
|
||||
wiz.provider = 'supabase'
|
||||
// The clears above are for a fresh run. This one is the same run coming back from the
|
||||
// redirect, so what it had already created is still its own to write over.
|
||||
claims = claimsFromJSON(resume.claims)
|
||||
createdProjects = resume.createdProjects ?? []
|
||||
claims = claimsFromJSON(from.claims)
|
||||
createdProjects = from.createdProjects ?? []
|
||||
leftBehind = anythingClaimed(claims) || createdProjects.length > 0
|
||||
// Which side of the toggle it was on, and the organization it was pointed at. Left to
|
||||
// default, a run that died mid-create comes back asking for the password it generated.
|
||||
if (resume.mode) wiz.supabase.mode = resume.mode
|
||||
if (resume.org) wiz.supabase.org = resume.org
|
||||
if (from.mode) wiz.supabase.mode = from.mode
|
||||
if (from.org) wiz.supabase.org = from.org
|
||||
// The password is deliberately not parked -- it is a secret and sessionStorage is not
|
||||
// the place for one. Carrying the project is what stops the resume landing on a
|
||||
// different database with an empty password field and no sign anything moved.
|
||||
if (resume.project) wiz.supabase.project = resume.project
|
||||
if (resume.connectionMode) wiz.supabase.connectionMode = resume.connectionMode
|
||||
const cut = resume.resourcePath?.lastIndexOf('/') ?? -1
|
||||
if (resume.resourcePath && cut > 0) {
|
||||
wiz.review.folder = resume.resourcePath.slice(0, cut)
|
||||
wiz.review.resourceName = resume.resourcePath.slice(cut + 1)
|
||||
if (from.project) wiz.supabase.project = from.project
|
||||
if (from.connectionMode) wiz.supabase.connectionMode = from.connectionMode
|
||||
const cut = from.resourcePath?.lastIndexOf('/') ?? -1
|
||||
if (from.resourcePath && cut > 0) {
|
||||
wiz.review.folder = from.resourcePath.slice(0, cut)
|
||||
wiz.review.resourceName = from.resourcePath.slice(cut + 1)
|
||||
}
|
||||
enterStep(2)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Opened by the settings page. Reopening after the Supabase redirect drops the user back on
|
||||
* the setup step with what they had already chosen, so authorizing does not feel like
|
||||
* starting over.
|
||||
* Opened by the settings page. A run coming back from the Supabase redirect is handed in
|
||||
* rather than read off the `resume` prop: the caller has it, and taking it as an argument is
|
||||
* what keeps the restore independent of when the prop it was assigned to reaches this
|
||||
* component.
|
||||
*/
|
||||
export function open() {
|
||||
reset()
|
||||
export function open(parked?: WizardResume) {
|
||||
reset(parked ?? resume)
|
||||
opened = true
|
||||
}
|
||||
|
||||
|
||||
@@ -224,7 +224,7 @@
|
||||
const wizardEnabled = isDataTableWizardEnabled()
|
||||
let wizardOpen = $state(false)
|
||||
/** Opened through the wizard's own `open()`, which is what sets a fresh run up. */
|
||||
let wizard: { open: () => void } | undefined = $state(undefined)
|
||||
let wizard: { open: (parked?: WizardResume) => void } | undefined = $state(undefined)
|
||||
let wizardResume: WizardResume | undefined = $state(undefined)
|
||||
|
||||
// Supabase sends the user back here after authorizing; pick the wizard back up where it
|
||||
@@ -234,7 +234,9 @@
|
||||
const parked = takeParkedWizard()
|
||||
if (parked) {
|
||||
wizardResume = parked
|
||||
wizard?.open()
|
||||
// Handed in, not left to the `resume` prop: the wizard rebuilds the run synchronously
|
||||
// inside this call, and a parked run that arrived late would come back as a fresh one.
|
||||
wizard?.open(parked)
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -368,7 +368,7 @@ export type RunDeps = {
|
||||
* name or the path in between.
|
||||
*/
|
||||
claims: Claims
|
||||
/** The mark written onto a secret or resource this run creates: `edited_by` / `created_by`. */
|
||||
/** Stands in as the mark where the object was written but its timestamp could not be read back. */
|
||||
username: string
|
||||
}
|
||||
|
||||
|
||||
@@ -66,7 +66,7 @@ export function anythingClaimed(claims: Claims): boolean {
|
||||
|
||||
/**
|
||||
* Carried across the full-page redirect the blocked-popup Supabase leg falls back to. No secret
|
||||
* travels: a mark is a username or a resource path.
|
||||
* travels: a mark is a timestamp or a resource path.
|
||||
*/
|
||||
export function claimsToJSON(claims: Claims): Claim[] {
|
||||
return [...claims]
|
||||
|
||||
@@ -138,6 +138,26 @@ describe('unsupportedConnectionParam', () => {
|
||||
expect(parsePostgresConnectionString(disguised)?.sslmode).toBeUndefined()
|
||||
})
|
||||
|
||||
// libpq matches parameter names case-insensitively. Folding in one reader and not the other
|
||||
// is what lets a name through the allowlist and past the parser, so the string is saved as
|
||||
// whatever the default happens to be rather than what it asked for.
|
||||
it('reads a parameter whatever its case', () => {
|
||||
const shouted = 'postgres://u:p@h/db?SslMode=verify-full'
|
||||
expect(unsupportedConnectionParam(shouted)).toBeUndefined()
|
||||
expect(parsePostgresConnectionString(shouted)?.sslmode).toBe('verify-full')
|
||||
expect(unsupportedConnectionParam('postgres://u:p@h/db?Connect_Timeout=1')).toBe(
|
||||
'connect_timeout'
|
||||
)
|
||||
})
|
||||
|
||||
// libpq takes the last of a repeated parameter. Taking the first reads a weaker mode than
|
||||
// the string actually asks for.
|
||||
it('takes the last value of a repeated parameter', () => {
|
||||
expect(
|
||||
parsePostgresConnectionString('postgres://u:p@h/db?sslmode=disable&sslmode=require')?.sslmode
|
||||
).toBe('require')
|
||||
})
|
||||
|
||||
it('ignores the one it can store, and the ones that cost nothing', () => {
|
||||
expect(unsupportedConnectionParam('postgres://u:p@h/db?sslmode=require')).toBeUndefined()
|
||||
expect(unsupportedConnectionParam('postgres://u:p@h/db?application_name=wm')).toBeUndefined()
|
||||
|
||||
@@ -24,9 +24,18 @@
|
||||
const CONNECTION_STRING =
|
||||
/postgres(?:ql)?:\/\/(?<user>[^:@]+)(?::(?<password>[^@]+))?@(?<host>\[[^\]]+\]|[^:\/?]+)(?::(?<port>\d+))?\/(?<dbname>[^\?]+)?/
|
||||
|
||||
/** Everything after the first `?`, which is where every query parameter lives. */
|
||||
function queryOf(connectionString: string): string {
|
||||
return connectionString.split('?').slice(1).join('?')
|
||||
/**
|
||||
* The query parameters, read the way libpq reads them: names are case-insensitive, and a name
|
||||
* repeated takes its last value. One reader for both the parser and the allowlist below, or
|
||||
* they disagree about what a string says — a name only one of them folds is refused by neither
|
||||
* and honoured by neither.
|
||||
*/
|
||||
function paramsOf(connectionString: string): Map<string, string> {
|
||||
const query = connectionString.split('?').slice(1).join('?')
|
||||
const params = new Map<string, string>()
|
||||
if (!query) return params
|
||||
new URLSearchParams(query).forEach((value, name) => params.set(name.toLowerCase(), value))
|
||||
return params
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -63,8 +72,7 @@ export function parsePostgresConnectionString(
|
||||
// By parameter name, never by searching the query text: `sslmode=` also occurs inside
|
||||
// another parameter's *value*, and a substring match there reads someone's
|
||||
// `application_name=sslmode=disable` as a request to turn TLS off.
|
||||
const query = queryOf(connectionString)
|
||||
const sslmode = query ? new URLSearchParams(query).get('sslmode') : null
|
||||
const sslmode = paramsOf(connectionString).get('sslmode')
|
||||
return {
|
||||
user: decode(user),
|
||||
password: password ? decode(password) : undefined,
|
||||
@@ -92,15 +100,10 @@ const COSMETIC_PARAMS = ['application_name']
|
||||
* other than the one pasted, behind a probe that reports success.
|
||||
*/
|
||||
export function unsupportedConnectionParam(connectionString: string): string | undefined {
|
||||
const query = queryOf(connectionString)
|
||||
if (!query) return undefined
|
||||
let found: string | undefined = undefined
|
||||
new URLSearchParams(query).forEach((_value, name) => {
|
||||
const known = REPRESENTABLE_PARAMS.includes(name.toLowerCase())
|
||||
const harmless = COSMETIC_PARAMS.includes(name.toLowerCase())
|
||||
if (!found && !known && !harmless) found = name
|
||||
})
|
||||
return found
|
||||
for (const name of paramsOf(connectionString).keys()) {
|
||||
if (!REPRESENTABLE_PARAMS.includes(name) && !COSMETIC_PARAMS.includes(name)) return name
|
||||
}
|
||||
return undefined
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user