add an endpoint for doing SQL selects (#641)

* add an endpoint for doing SQL selects

This adds an endpoint for doing SQL selects directly on a Grist document. Other kinds of statements are not supported. There is a default timeout of a second on queries.

This follows loosely an API design by Alex Hall.

Co-authored-by: jarek <jaroslaw.sadzinski@gmail.com>

app/gen-server/lib/DocApiForwarder.ts

@@ -61,6 +61,7 @@ export class DocApiForwarder {
     app.use('/api/docs/:docId/webhooks/queue', withDoc);
     app.use('/api/docs/:docId/webhooks', withDoc);
     app.use('/api/docs/:docId/assistant', withDoc);
+    app.use('/api/docs/:docId/sql', withDoc);
     app.use('^/api/docs$', withoutDoc);
   }
 

app/plugin/DocApiTypes-ti.ts

@@ -84,6 +84,12 @@ export const TablesPatch = t.iface([], {
   "tables": t.tuple("RecordWithStringId", t.rest(t.array("RecordWithStringId"))),
 });
 
+export const SqlPost = t.iface([], {
+  "sql": "string",
+  "args": t.opt(t.array("any")),
+  "timeout": t.opt("number"),
+});
+
 const exportedTypeSuite: t.ITypeSuite = {
   NewRecord,
   NewRecordWithStringId,
@@ -101,5 +107,6 @@ const exportedTypeSuite: t.ITypeSuite = {
   TablePost,
   TablesPost,
   TablesPatch,
+  SqlPost,
 };
 export default exportedTypeSuite;

app/plugin/DocApiTypes.ts

@@ -107,3 +107,16 @@ export interface TablesPost {
 export interface TablesPatch {
   tables: [RecordWithStringId, ...RecordWithStringId[]]; // at least one table is required
 }
+
+/**
+ * JSON schema for the body of api /sql POST endpoint
+ */
+export interface SqlPost {
+  sql: string;
+  args?: any[];      // (It would be nice to support named parameters, but
+                     // that feels tricky currently with node-sqlite3)
+  timeout?: number;  // In msecs. Can only be reduced from server default,
+                     // not increased. Note timeout of a query could affect
+                     // other queued queries on same document, because of
+                     // limitations of API node-sqlite3 exposes.
+}

app/server/lib/ActiveDoc.ts

@@ -914,6 +914,11 @@ export class ActiveDoc extends EventEmitter {
     return this._granularAccess.canCopyEverything(docSession);
   }
 
+  // Check if user has rights to read everything in this doc.
+  public async canCopyEverything(docSession: OptDocSession) {
+    return this._granularAccess.canCopyEverything(docSession);
+  }
+
   // Check if it is appropriate for the user to be treated as an owner of
   // the document for granular access purposes when in "prefork" mode
   // (meaning a document has been opened with the intent to fork it, but

app/server/lib/AppSettings.ts

@@ -96,6 +96,17 @@ export class AppSettings {
     return result;
   }
 
+  /**
+   * As for readInt() but fail if nothing was found.
+   */
+  public requireInt(query: AppSettingQuery): number {
+    const result = this.readInt(query);
+    if (result === undefined) {
+      throw new Error(`missing environment variable: ${query.envVar}`);
+    }
+    return result;
+  }
+
   /**
    * As for read() but type (and store, and report) the result as
    * a boolean.
@@ -107,6 +118,17 @@ export class AppSettings {
     return result;
   }
 
+  /**
+   * As for read() but type (and store, and report) the result as
+   * an integer (well, a number).
+   */
+  public readInt(query: AppSettingQuery): number|undefined {
+    this.readString(query);
+    const result = this.getAsInt();
+    this._value = result;
+    return result;
+  }
+
   /* set this setting 'manually' */
   public set(value: JSONValue): void {
     this._value = value;

app/server/lib/DocApi.ts

@@ -32,6 +32,7 @@ import {
   TableOperationsPlatform
 } from 'app/plugin/TableOperationsImpl';
 import {ActiveDoc, colIdToRef as colIdToReference, tableIdToRef} from "app/server/lib/ActiveDoc";
+import {appSettings} from "app/server/lib/AppSettings";
 import {sendForCompletion} from 'app/server/lib/Assistance';
 import {
   assertAccess,
@@ -96,16 +97,25 @@ const MAX_PARALLEL_REQUESTS_PER_DOC = 10;
 // then the _dailyUsage cache may become unreliable and users may be able to exceed their allocated requests.
 const MAX_ACTIVE_DOCS_USAGE_CACHE = 1000;
 
+// Maximum duration of a call to /sql. Does not apply to internal calls to SQLite.
+const MAX_CUSTOM_SQL_MSEC = appSettings.section('integrations')
+  .section('sql').flag('timeout').requireInt({
+    envVar: 'GRIST_SQL_TIMEOUT_MSEC',
+    defaultValue: 1000,
+  });
+
 type WithDocHandler = (activeDoc: ActiveDoc, req: RequestWithLogin, resp: Response) => Promise<void>;
 
 // Schema validators for api endpoints that creates or updates records.
 const {
   RecordsPatch, RecordsPost, RecordsPut,
   ColumnsPost, ColumnsPatch, ColumnsPut,
+  SqlPost,
   TablesPost, TablesPatch,
 } = t.createCheckers(DocApiTypesTI, GristDataTI);
 
-for (const checker of [RecordsPatch, RecordsPost, RecordsPut, ColumnsPost, ColumnsPatch, TablesPost, TablesPatch]) {
+for (const checker of [RecordsPatch, RecordsPost, RecordsPut, ColumnsPost, ColumnsPatch,
+                       SqlPost, TablesPost, TablesPatch]) {
   checker.setReportedPath("body");
 }
 
@@ -518,6 +528,27 @@ export class DocWorkerApi {
       })
     );
 
+    // A GET /sql endpoint that takes a query like ?q=select+*+from+Table1
+    // Not very useful, apart from testing - see the POST endpoint for
+    // serious use.
+    // If SQL statements that modify the DB are ever supported, they should
+    // not be permitted by this endpoint.
+    this._app.get(
+      '/api/docs/:docId/sql', canView,
+      withDoc(async (activeDoc, req, res) => {
+        const sql = stringParam(req.query.q, 'q');
+        await this._runSql(activeDoc, req, res, { sql });
+      }));
+
+    // A POST /sql endpoint, accepting a body like:
+    // { "sql": "select * from Table1 where name = ?", "args": ["Paul"] }
+    // Only SELECT statements are currently supported.
+    this._app.post(
+      '/api/docs/:docId/sql', canView, validate(SqlPost),
+      withDoc(async (activeDoc, req, res) => {
+        await this._runSql(activeDoc, req, res, req.body);
+      }));
+
     // Create columns in a table, given as records of the _grist_Tables_column metatable.
     this._app.post('/api/docs/:docId/tables/:tableId/columns', canEdit, validate(ColumnsPost),
       withDoc(async (activeDoc, req, res) => {
@@ -1502,6 +1533,73 @@ export class DocWorkerApi {
     await this._dbManager.flushSingleDocAuthCache(scope, docId);
     await this._docManager.interruptDocClients(docId);
   }
+
+  private async _runSql(activeDoc: ActiveDoc, req: RequestWithLogin, res: Response,
+      options: Types.SqlPost) {
+    if (!await activeDoc.canCopyEverything(docSessionFromRequest(req))) {
+      throw new ApiError('insufficient document access', 403);
+    }
+    const statement = options.sql;
+    // A very loose test, just for early error message
+    if (!(statement.toLowerCase().includes('select'))) {
+      throw new ApiError('only select statements are supported', 400);
+    }
+    const sqlOptions = activeDoc.docStorage.getOptions();
+    if (!sqlOptions?.canInterrupt || !sqlOptions?.bindableMethodsProcessOneStatement) {
+      throw new ApiError('The available SQLite wrapper is not adequate', 500);
+    }
+    const timeout =
+        Math.max(0, Math.min(MAX_CUSTOM_SQL_MSEC,
+                             optIntegerParam(options.timeout) || MAX_CUSTOM_SQL_MSEC));
+    // Wrap in a select to commit to the SELECT branch of SQLite
+    // grammar. Note ; isn't a problem.
+    //
+    // The underlying SQLite functions used will only process the
+    // first statement in the supplied text. For node-sqlite3, the
+    // remainder is placed in a "tail string" ignored by that library.
+    // So a Robert'); DROP TABLE Students;-- style attack isn't applicable.
+    //
+    // Since Grist is used with multiple SQLite wrappers, not just
+    // node-sqlite3, we have added a bindableMethodsProcessOneStatement
+    // flag that will need adding for each wrapper, and this endpoint
+    // will not operate unless that flag is set to true.
+    //
+    // The text is wrapped in select * from (USER SUPPLIED TEXT) which
+    // puts SQLite unconditionally onto the SELECT branch of its
+    // grammar. It is straightforward to break out of such a wrapper
+    // with multiple statements, but again, only the first statement
+    // is processed.
+    const wrappedStatement = `select * from (${statement})`;
+    const interrupt = setTimeout(async () => {
+      await activeDoc.docStorage.interrupt();
+    }, timeout);
+    try {
+      const records = await activeDoc.docStorage.all(wrappedStatement,
+                                                     ...(options.args || []));
+      res.status(200).json({
+        statement,
+        records: records.map(
+          rec => ({
+            fields: rec,
+          })
+        ),
+      });
+    } catch (e) {
+      if (e?.code === 'SQLITE_INTERRUPT') {
+        res.status(400).json({
+          error: "a slow statement resulted in a database interrupt",
+        });
+      } else if (e?.code === 'SQLITE_ERROR') {
+        res.status(400).json({
+          error: e?.message,
+        });
+      } else {
+        throw e;
+      }
+    } finally {
+      clearTimeout(interrupt);
+    }
+  }
 }
 
 export function addDocApiRoutes(

app/server/lib/DocStorage.ts

@@ -31,6 +31,7 @@ import {ISQLiteDB, MigrationHooks, OpenMode, PreparedStatement, quoteIdent,
 import chunk = require('lodash/chunk');
 import cloneDeep = require('lodash/cloneDeep');
 import groupBy = require('lodash/groupBy');
+import { MinDBOptions } from './SqliteCommon';
 
 
 // Run with environment variable NODE_DEBUG=db (may include additional comma-separated sections)
@@ -1419,6 +1420,14 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage {
     }
   }
 
+  public interrupt(): Promise<void> {
+    return this._getDB().interrupt();
+  }
+
+  public getOptions(): MinDBOptions|undefined {
+    return this._getDB().getOptions();
+  }
+
   public all(sql: string, ...args: any[]): Promise<ResultRow[]> {
     return this._getDB().all(sql, ...args);
   }

app/server/lib/SQLiteDB.ts

@@ -72,7 +72,7 @@ import {timeFormat} from 'app/common/timeFormat';
 import {create} from 'app/server/lib/create';
 import * as docUtils from 'app/server/lib/docUtils';
 import log from 'app/server/lib/log';
-import {MinDB, MinRunResult, PreparedStatement, ResultRow,
+import {MinDB, MinDBOptions, MinRunResult, PreparedStatement, ResultRow,
         SqliteVariant, Statement} from 'app/server/lib/SqliteCommon';
 import {NodeSqliteVariant} from 'app/server/lib/SqliteNode';
 import assert from 'assert';
@@ -258,6 +258,14 @@ export class SQLiteDB implements ISQLiteDB {
   private constructor(protected _db: MinDB, private _dbPath: string) {
   }
 
+  public async interrupt(): Promise<void> {
+    return this._db.interrupt?.();
+  }
+
+  public getOptions(): MinDBOptions|undefined {
+    return this._db.getOptions?.();
+  }
+
   public async all(sql: string, ...args: any[]): Promise<ResultRow[]> {
     const result = await this._db.all(sql, ...args);
     return result;

app/server/lib/SqliteCommon.ts

@@ -12,20 +12,50 @@ import { OpenMode, quoteIdent } from 'app/server/lib/SQLiteDB';
 // eslint-disable-next-line @typescript-eslint/no-empty-interface
 export interface Statement {}
 
+// Some facts about the wrapper implementation.
+export interface MinDBOptions {
+  // is interruption implemented?
+  canInterrupt: boolean;
+
+  // Do all methods apart from exec() process at most one
+  // statement?
+  bindableMethodsProcessOneStatement: boolean;
+}
+
 export interface MinDB {
+  // This method is expected to be able to handle multiple
+  // semicolon-separated statements, as for sqlite3_exec:
+  //   https://www.sqlite.org/c3ref/exec.html
   exec(sql: string): Promise<void>;
+
+  // For all these methods, sql should ultimately be passed
+  // to sqlite3_prepare_v2 or later, and any tail text ignored after
+  // the first complete statement, so only the first statement is
+  // used if there are multiple.
+  //   https://www.sqlite.org/c3ref/prepare.html
   run(sql: string, ...params: any[]): Promise<MinRunResult>;
   get(sql: string, ...params: any[]): Promise<ResultRow|undefined>;
   all(sql: string, ...params: any[]): Promise<ResultRow[]>;
   prepare(sql: string, ...params: any[]): Promise<PreparedStatement>;
   runAndGetId(sql: string, ...params: any[]): Promise<number>;
-  close(): Promise<void>;
   allMarshal(sql: string, ...params: any[]): Promise<Buffer>;
 
+  close(): Promise<void>;
+
   /**
    * Limit the number of ATTACHed databases permitted.
    */
   limitAttach(maxAttach: number): Promise<void>;
+
+  /**
+   * Stop all current queries.
+   */
+  interrupt?(): Promise<void>;
+
+  /**
+   * Get some facts about the wrapper.
+   */
+  getOptions?(): MinDBOptions;
 }
 
 export interface MinRunResult {

app/server/lib/SqliteNode.ts

@@ -1,6 +1,6 @@
 import * as sqlite3 from '@gristlabs/sqlite3';
 import { fromCallback } from 'app/server/lib/serverUtils';
-import { MinDB, PreparedStatement, ResultRow, SqliteVariant } from 'app/server/lib/SqliteCommon';
+import { MinDB, MinDBOptions, PreparedStatement, ResultRow, SqliteVariant } from 'app/server/lib/SqliteCommon';
 import { OpenMode, RunResult } from 'app/server/lib/SQLiteDB';
 
 export class NodeSqliteVariant implements SqliteVariant {
@@ -84,6 +84,17 @@ export class NodeSqlite3DatabaseAdapter implements MinDB {
     this._db.close();
   }
 
+  public async interrupt(): Promise<void> {
+    this._db.interrupt();
+  }
+
+  public getOptions(): MinDBOptions {
+    return {
+      canInterrupt: true,
+      bindableMethodsProcessOneStatement: true,
+    };
+  }
+
   public async allMarshal(sql: string, ...params: any[]): Promise<Buffer> {
     // allMarshal isn't in the typings, because it is our addition to our fork of sqlite3 JS lib.
     return fromCallback(cb => (this._db as any).allMarshal(sql, ...params, cb));

test/server/lib/DocApi.ts

@@ -874,6 +874,15 @@ function testDocApi() {
     resp = await axios.post(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/_grist_Tables/data/delete`,
       [2, 3, 4, 5, 6], chimpy);
     assert.equal(resp.status, 200);
+
+    // Despite deleting tables (even in a more official way than above),
+    // there are rules lingering relating to them. TODO: look into this.
+    resp = await axios.post(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/_grist_ACLRules/data/delete`,
+      [2, 3], chimpy);
+    assert.equal(resp.status, 200);
+    resp = await axios.post(`${serverUrl}/api/docs/${docIds.Timesheets}/tables/_grist_ACLResources/data/delete`,
+      [2, 3], chimpy);
+    assert.equal(resp.status, 200);
   });
 
   describe("/docs/{did}/tables/{tid}/columns", function () {
@@ -3174,6 +3183,7 @@ function testDocApi() {
         users: {"kiwi@getgrist.com": 'editors' as string | null}
       };
       let accessResp = await axios.patch(`${homeUrl}/api/docs/${docIds.Timesheets}/access`, {delta}, chimpy);
+      await flushAuth();
       assert.equal(accessResp.status, 200);
 
       const check = userCheck.bind(null, kiwi);
@@ -3198,6 +3208,7 @@ function testDocApi() {
       delta.users['kiwi@getgrist.com'] = null;
       accessResp = await axios.patch(`${homeUrl}/api/docs/${docIds.Timesheets}/access`, {delta}, chimpy);
       assert.equal(accessResp.status, 200);
+      await flushAuth();
     });
 
     it("DELETE /docs/{did}/tables/webhooks should not be allowed for not-owner", async function () {
@@ -3210,6 +3221,7 @@ function testDocApi() {
       };
       let accessResp = await axios.patch(`${homeUrl}/api/docs/${docIds.Timesheets}/access`, {delta}, chimpy);
       assert.equal(accessResp.status, 200);
+      await flushAuth();
 
       // Actually unsubscribe with the same unsubscribeKey that was returned by registration - it shouldn't be accepted
       await check(subscribeResponse.webhookId, 403, /No owner access/);
@@ -3219,6 +3231,7 @@ function testDocApi() {
       delta.users['kiwi@getgrist.com'] = null;
       accessResp = await axios.patch(`${homeUrl}/api/docs/${docIds.Timesheets}/access`, {delta}, chimpy);
       assert.equal(accessResp.status, 200);
+      await flushAuth();
     });
   });
 
@@ -4502,6 +4515,160 @@ function testDocApi() {
 
   });
 
+
+  it ("GET /docs/{did}/sql is functional", async function () {
+    const query = 'select+*+from+Table1+order+by+id';
+    const resp = await axios.get(`${homeUrl}/api/docs/${docIds.Timesheets}/sql?q=${query}`, chimpy);
+    assert.equal(resp.status, 200);
+    assert.deepEqual(resp.data, {
+      statement: 'select * from Table1 order by id',
+      records: [
+        {
+          fields: {
+            id: 1,
+            manualSort: 1,
+            A: 'hello',
+            B: '',
+            C: '',
+            D: null,
+            E: 'HELLO'
+          },
+        },
+        {
+          fields: { id: 2, manualSort: 2, A: '', B: 'world', C: '', D: null, E: '' }
+        },
+        {
+          fields: { id: 3, manualSort: 3, A: '', B: '', C: '', D: null, E: '' }
+        },
+        {
+          fields: { id: 4, manualSort: 4, A: '', B: '', C: '', D: null, E: '' }
+        },
+      ]
+    });
+  });
+
+  it ("POST /docs/{did}/sql is functional", async function () {
+    let resp = await axios.post(
+      `${homeUrl}/api/docs/${docIds.Timesheets}/sql`,
+      { sql: "select A from Table1 where id = ?", args: [ 1 ] },
+      chimpy);
+    assert.equal(resp.status, 200);
+    assert.deepEqual(resp.data.records, [{
+      fields: {
+        A: 'hello',
+      }
+    }]);
+
+    resp = await axios.post(
+      `${homeUrl}/api/docs/${docIds.Timesheets}/sql`,
+      { nosql: "select A from Table1 where id = ?", args: [ 1 ] },
+      chimpy);
+    assert.equal(resp.status, 400);
+    assert.deepEqual(resp.data, {
+      error: 'Invalid payload',
+      details: { userError: 'Error: body.sql is missing' }
+    });
+  });
+
+  it ("POST /docs/{did}/sql has access control", async function () {
+    // Check non-viewer doesn't have access.
+    const url = `${homeUrl}/api/docs/${docIds.Timesheets}/sql`;
+    const query = { sql: "select A from Table1 where id = ?", args: [ 1 ] };
+    let resp = await axios.post(url, query, kiwi);
+    assert.equal(resp.status, 403);
+    assert.deepEqual(resp.data, {
+      error: 'No view access',
+    });
+
+    try {
+      // Check a viewer would have access.
+      const delta = {
+        users: { 'kiwi@getgrist.com': 'viewers' },
+      };
+      await axios.patch(`${homeUrl}/api/docs/${docIds.Timesheets}/access`, {delta}, chimpy);
+      await flushAuth();
+      resp = await axios.post(url, query, kiwi);
+      assert.equal(resp.status, 200);
+
+      // Check a viewer would not have access if there is some private material.
+      await axios.post(
+        `${homeUrl}/api/docs/${docIds.Timesheets}/apply`, [
+          ['AddTable', 'TablePrivate', [{id: 'A', type: 'Int'}]],
+          ['AddRecord', '_grist_ACLResources', -1, {tableId: 'TablePrivate', colIds: '*'}],
+          ['AddRecord', '_grist_ACLRules', null, {
+            resource: -1, aclFormula: '', permissionsText: 'none',
+          }],
+        ], chimpy);
+      resp = await axios.post(url, query, kiwi);
+      assert.equal(resp.status, 403);
+    } finally {
+      // Remove extra viewer; remove extra table.
+      const delta = {
+        users: { 'kiwi@getgrist.com': null },
+      };
+      await axios.patch(`${homeUrl}/api/docs/${docIds.Timesheets}/access`, {delta}, chimpy);
+      await flushAuth();
+      await axios.post(
+        `${homeUrl}/api/docs/${docIds.Timesheets}/apply`, [
+          ['RemoveTable', 'TablePrivate'],
+        ], chimpy);
+    }
+  });
+
+  it ("POST /docs/{did}/sql accepts only selects", async function () {
+    async function check(accept: boolean, sql: string, ...args: any[]) {
+      const resp = await axios.post(
+        `${homeUrl}/api/docs/${docIds.Timesheets}/sql`,
+        { sql, args },
+        chimpy);
+      if (accept) {
+        assert.equal(resp.status, 200);
+      } else {
+        assert.equal(resp.status, 400);
+      }
+      return resp.data;
+    }
+    await check(true, 'select * from Table1');
+    await check(true, '  SeLeCT * from Table1');
+    await check(true, 'with results as (select 1) select * from results');
+
+    // rejected quickly since no select
+    await check(false, 'delete from Table1');
+    await check(false, '');
+
+    // rejected because deletes/updates/... can't be nested within a select
+    await check(false, "delete from Table1 where id in (select id from Table1) and 'selecty' = 'selecty'");
+    await check(false, "update Table1 set A = ? where 'selecty' = 'selecty'", 'test');
+    await check(false, "pragma data_store_directory = 'selecty'");
+    await check(false, "create table selecty(x, y)");
+    await check(false, "attach database 'selecty' AS test");
+
+    // rejected because ";" can't be nested
+    await check(false, 'select * from Table1; delete from Table1');
+
+    // Of course, we can get out of the wrapping select, but we can't
+    // add on more statements. For example, the following runs with no
+    // trouble - but only the SELECT part. The DELETE is discarded.
+    // (node-sqlite3 doesn't expose enough to give an error message for
+    // this, though we could extend it).
+    await check(true, 'select * from Table1); delete from Table1 where id in (select id from Table1');
+    const {records} = await check(true, 'select * from Table1');
+    // Double-check the deletion didn't happen.
+    assert.lengthOf(records, 4);
+  });
+
+  it ("POST /docs/{did}/sql timeout is effective", async function () {
+    const slowQuery = 'WITH RECURSIVE r(i) AS (VALUES(0) ' +
+        'UNION ALL SELECT i FROM r  LIMIT 1000000) ' +
+        'SELECT i FROM r WHERE i = 1';
+    const resp = await axios.post(
+      `${homeUrl}/api/docs/${docIds.Timesheets}/sql`,
+      { sql: slowQuery, timeout: 10 },
+      chimpy);
+    assert.equal(resp.status, 400);
+    assert.match(resp.data.error, /database interrupt/);
+  });
+
   // PLEASE ADD MORE TESTS HERE
 }
 
@@ -4558,3 +4725,10 @@ async function setupDataDir(dir: string) {
     'ApiDataRecordsTest.grist',
     path.resolve(dir, docIds.ApiDataRecordsTest + '.grist'));
 }
+
+// The access control level of a user on a document may be cached for a
+// few seconds. This method flushes that cache.
+async function flushAuth() {
+  await home.testingHooks.flushAuthorizerCache();
+  await docs.testingHooks.flushAuthorizerCache();
+}