From 51756881ec14c9e8a9ea90ec1f0909704352291d Mon Sep 17 00:00:00 2001 From: James Swirhun Date: Fri, 16 Oct 2026 17:29:07 -0600 Subject: [PATCH 1/4] fix(server): read a package data file whose name has a space Signed-off-by: James Swirhun --- packages/server/src/service/package.spec.ts | 38 +++++++++++++++++++++ packages/server/src/service/package.ts | 9 ++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/packages/server/src/service/package.spec.ts b/packages/server/src/service/package.spec.ts index 003d562cc..23909ec13 100644 --- a/packages/server/src/service/package.spec.ts +++ b/packages/server/src/service/package.spec.ts @@ -415,6 +415,44 @@ describe("service/package", () => { }); }); + it("should probe a data file whose name has a space or a quote", async () => { + // A bare path with a space is refused by DuckDB's table-path + // check, so these files used to list with `error` and no `info`. + await fs.mkdir(join(testPackageDirectory, "data"), { + recursive: true, + }); + const csv = "Name,Value\nJohn,10\nJane,20\nJim,30"; + await fs.writeFile( + join(testPackageDirectory, "data", "Revenue by month.csv"), + csv, + ); + await fs.writeFile( + join(testPackageDirectory, "data", "Bob's sales.csv"), + csv, + ); + const conn = new DuckDBConnection("duckdb"); + + for (const name of [ + "data/Revenue by month.csv", + "data/Bob's sales.csv", + ]) { + // @ts-expect-error Accessing private static method for testing + const info = await Package.getDatabaseInfo( + testPackageDirectory, + name, + conn, + ); + expect(info).toEqual({ + name, + columns: [ + { name: "Name", type: "string" }, + { name: "Value", type: "number" }, + ], + rowCount: 3, + }); + } + }); + it("should probe an xlsx database file", async () => { // Real committed fixture: DuckDB's excel extension reads it via the // same duckdb.table() replacement scan the csv path uses. diff --git a/packages/server/src/service/package.ts b/packages/server/src/service/package.ts index 27abbbf9c..f792d7407 100644 --- a/packages/server/src/service/package.ts +++ b/packages/server/src/service/package.ts @@ -3325,8 +3325,15 @@ export class Package { }); // Forward slashes work on Windows DuckDB and avoid escaping. const normalizedPath = fullPath.replace(/\\/g, "/"); + // DuckDB's table-path check refuses a bare path holding a space or a + // quote, which left such a file listed as unreadable. So the path goes + // in as a single-quoted literal ('' for a quote), which the check + // accepts and canonicalizes to the same SQL a bare path gets. The + // literal sits in a Malloy double-quoted string, so `\` and `"` are + // escaped for that string. + const literal = `'${normalizedPath.replace(/'/g, "''")}'`; const model = runtime.loadModel( - `source: temp is duckdb.table('${normalizedPath}')`, + `source: temp is duckdb.table("${literal.replace(/[\\"]/g, (c) => `\\${c}`)}")`, ); const modelDef = await model.getModel(); const fields = (modelDef._modelDef.contents["temp"] as SourceDef).fields; From f3d5c51ebed9f61e3f152d283cebb127c4bc403f Mon Sep 17 00:00:00 2001 From: James Swirhun Date: Sun, 18 Oct 2026 09:22:33 -0600 Subject: [PATCH 2/4] test(server): probe data files named with parens, non-ASCII and a double quote, not only a space and an apostrophe Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HtaPGjDSK4pUZ3pr7c17ZG Signed-off-by: James Swirhun --- packages/server/src/service/package.spec.ts | 33 +++++++++++---------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/server/src/service/package.spec.ts b/packages/server/src/service/package.spec.ts index 23909ec13..4b3534cf7 100644 --- a/packages/server/src/service/package.spec.ts +++ b/packages/server/src/service/package.spec.ts @@ -415,27 +415,30 @@ describe("service/package", () => { }); }); - it("should probe a data file whose name has a space or a quote", async () => { - // A bare path with a space is refused by DuckDB's table-path - // check, so these files used to list with `error` and no `info`. + it("should probe a data file whose name is outside the bare-path set", async () => { + // DuckDB's table-path check takes a bare path only when every + // character is in a short safe set, so these files used to list + // with `error` and no `info`. A double quote can't appear in a + // Windows file name. + const names = [ + "Revenue by month.csv", + "Bob's sales.csv", + "orders (1).csv", + "données.csv", + ...(process.platform === "win32" ? [] : ['say "hi".csv']), + ]; await fs.mkdir(join(testPackageDirectory, "data"), { recursive: true, }); const csv = "Name,Value\nJohn,10\nJane,20\nJim,30"; - await fs.writeFile( - join(testPackageDirectory, "data", "Revenue by month.csv"), - csv, - ); - await fs.writeFile( - join(testPackageDirectory, "data", "Bob's sales.csv"), - csv, - ); const conn = new DuckDBConnection("duckdb"); - for (const name of [ - "data/Revenue by month.csv", - "data/Bob's sales.csv", - ]) { + for (const file of names) { + await fs.writeFile( + join(testPackageDirectory, "data", file), + csv, + ); + const name = `data/${file}`; // @ts-expect-error Accessing private static method for testing const info = await Package.getDatabaseInfo( testPackageDirectory, From e950dd3fb4182a696c7444a39c0badd2c7505b35 Mon Sep 17 00:00:00 2001 From: James Swirhun Date: Mon, 19 Oct 2026 18:14:45 -0600 Subject: [PATCH 3/4] refactor(server): build the data file probe's SQL literal and Malloy string as two named steps, and pin that a name holding ; or -- is still refused Signed-off-by: James Swirhun --- packages/server/src/service/package.spec.ts | 24 +++++++++++++++++++++ packages/server/src/service/package.ts | 13 +++++------ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/packages/server/src/service/package.spec.ts b/packages/server/src/service/package.spec.ts index 4b3534cf7..d54ceb038 100644 --- a/packages/server/src/service/package.spec.ts +++ b/packages/server/src/service/package.spec.ts @@ -456,6 +456,30 @@ describe("service/package", () => { } }); + it("still refuses a data file whose name holds `;` or `--`", async () => { + // Malloy refuses these in a table path even when quoted, so the + // probe throws and the file lists with `error` and no `info`. Pinned + // so the day Malloy lifts the limit shows up as a test change. + await fs.mkdir(join(testPackageDirectory, "data"), { + recursive: true, + }); + const conn = new DuckDBConnection("duckdb"); + for (const file of ["a;b.csv", "a--b.csv"]) { + await fs.writeFile( + join(testPackageDirectory, "data", file), + "Name,Value\nJohn,10", + ); + await expect( + // @ts-expect-error Accessing private static method for testing + Package.getDatabaseInfo( + testPackageDirectory, + `data/${file}`, + conn, + ), + ).rejects.toThrow("forbidden character `;` or `--`"); + } + }); + it("should probe an xlsx database file", async () => { // Real committed fixture: DuckDB's excel extension reads it via the // same duckdb.table() replacement scan the csv path uses. diff --git a/packages/server/src/service/package.ts b/packages/server/src/service/package.ts index f792d7407..fa35b654c 100644 --- a/packages/server/src/service/package.ts +++ b/packages/server/src/service/package.ts @@ -3327,13 +3327,14 @@ export class Package { const normalizedPath = fullPath.replace(/\\/g, "/"); // DuckDB's table-path check refuses a bare path holding a space or a // quote, which left such a file listed as unreadable. So the path goes - // in as a single-quoted literal ('' for a quote), which the check - // accepts and canonicalizes to the same SQL a bare path gets. The - // literal sits in a Malloy double-quoted string, so `\` and `"` are - // escaped for that string. - const literal = `'${normalizedPath.replace(/'/g, "''")}'`; + // in as a single-quoted SQL literal ('' for a quote), which the check + // accepts and canonicalizes to the same SQL a bare path gets. + const sqlLiteral = `'${normalizedPath.replace(/'/g, "''")}'`; + // That literal sits in a Malloy double-quoted string. Normalizing left + // no backslash, so only `"` needs escaping there. + const malloyString = `"${sqlLiteral.replace(/"/g, '\\"')}"`; const model = runtime.loadModel( - `source: temp is duckdb.table("${literal.replace(/[\\"]/g, (c) => `\\${c}`)}")`, + `source: temp is duckdb.table(${malloyString})`, ); const modelDef = await model.getModel(); const fields = (modelDef._modelDef.contents["temp"] as SourceDef).fields; From d9ffbeccb41fd8ee382eaa3abf27b9d332e9e193 Mon Sep 17 00:00:00 2001 From: James Swirhun Date: Mon, 19 Oct 2026 19:48:22 -0600 Subject: [PATCH 4/4] fix(server): escape backslashes in the data file probe's Malloy string again, which CodeQL flags as incomplete escaping when only " is escaped Signed-off-by: James Swirhun --- packages/server/src/service/package.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/server/src/service/package.ts b/packages/server/src/service/package.ts index fa35b654c..9fe2ec0e2 100644 --- a/packages/server/src/service/package.ts +++ b/packages/server/src/service/package.ts @@ -3330,9 +3330,10 @@ export class Package { // in as a single-quoted SQL literal ('' for a quote), which the check // accepts and canonicalizes to the same SQL a bare path gets. const sqlLiteral = `'${normalizedPath.replace(/'/g, "''")}'`; - // That literal sits in a Malloy double-quoted string. Normalizing left - // no backslash, so only `"` needs escaping there. - const malloyString = `"${sqlLiteral.replace(/"/g, '\\"')}"`; + // That literal sits in a Malloy double-quoted string, so `\` and `"` are + // escaped for it. Normalizing above left no backslash, but escaping it + // keeps this step correct on its own. + const malloyString = `"${sqlLiteral.replace(/[\\"]/g, "\\$&")}"`; const model = runtime.loadModel( `source: temp is duckdb.table(${malloyString})`, );