From 11fe3e90d4abe5e6f7bd5c73d7d2e9b3711c9f01 Mon Sep 17 00:00:00 2001
From: Paul Fitzpatrick <paulfitz@alum.mit.edu>
Date: Mon, 30 Sep 2024 15:58:38 -0400
Subject: [PATCH] check sandbox viability lazily (#1226)

This checks whether code can successfully run in the
sandbox only when the admin panel needs to report that,
rather than at start up. This is motivated by two things:

  - The desktop app became a lot slower to open with this
    check, since it uses pyodide by default, and there's
    been no work on optimizing the pyodide sandbox load
    times (as opposed to gvisor, where a lot of work was
    done, and it is also fundamentally faster).
  - The messages logged by a test sandbox starting and
    stopping have been confusing people.

There is a case for doing the check on startup, especially
on servers, so that we can fail early. Still, that isn't
what we were doing, and we'd also like to move away from the
server refusing to start because of a problem and
towards an always-reachable admin page that reports
the nature of problems in a clearer way.
---
 app/client/ui/AdminPanel.ts   |  5 ++++-
 app/server/MergedServer.ts    |  6 ------
 app/server/lib/BootProbes.ts  |  2 +-
 app/server/lib/FlexServer.ts  | 12 ++++++------
 app/server/lib/GristServer.ts |  4 ++--
 5 files changed, 13 insertions(+), 16 deletions(-)

diff --git a/app/client/ui/AdminPanel.ts b/app/client/ui/AdminPanel.ts
index e2cc38bf..0b1e5404 100644
--- a/app/client/ui/AdminPanel.ts
+++ b/app/client/ui/AdminPanel.ts
@@ -202,7 +202,10 @@ Please log in as an administrator.`)),
         const success = result?.status === 'success';
         const details = result?.details as SandboxingBootProbeDetails|undefined;
         if (!details) {
-          return cssValueLabel(t('unknown'));
+          // Sandbox details get filled out relatively slowly if
+          // this is first time on admin panel. So show "checking"
+          // if we don't have a reported status yet.
+          return cssValueLabel(result?.status ? t('unknown') : t('checking'));
         }
         const flavor = details.flavor;
         const configured = details.configured;
diff --git a/app/server/MergedServer.ts b/app/server/MergedServer.ts
index ba307e80..2ac994d1 100644
--- a/app/server/MergedServer.ts
+++ b/app/server/MergedServer.ts
@@ -192,12 +192,6 @@ export class MergedServer {
       this.flexServer.checkOptionCombinations();
       this.flexServer.summary();
       this.flexServer.ready();
-
-      // Some tests have their timing perturbed by having this earlier
-      // TODO: update those tests.
-      if (this.hasComponent("docs")) {
-        await this.flexServer.checkSandbox();
-      }
     } catch(e) {
       await this.flexServer.close();
       throw e;
diff --git a/app/server/lib/BootProbes.ts b/app/server/lib/BootProbes.ts
index 36c3786c..df0f202e 100644
--- a/app/server/lib/BootProbes.ts
+++ b/app/server/lib/BootProbes.ts
@@ -265,7 +265,7 @@ const _sandboxingProbe: Probe = {
   id: 'sandboxing',
   name: 'Is document sandboxing effective',
   apply: async (server, req) => {
-    const details = server.getSandboxInfo();
+    const details = await server.getSandboxInfo();
     return {
       status: (details?.configured && details?.functional) ? 'success' : 'fault',
       details,
diff --git a/app/server/lib/FlexServer.ts b/app/server/lib/FlexServer.ts
index 4ce03fcb..2d8803eb 100644
--- a/app/server/lib/FlexServer.ts
+++ b/app/server/lib/FlexServer.ts
@@ -1397,8 +1397,9 @@ export class FlexServer implements GristServer {
     }
   }
 
-  public async checkSandbox() {
-    if (this._check('sandbox', 'doc')) { return; }
+  public async getSandboxInfo(): Promise<SandboxInfo> {
+    if (this._sandboxInfo) { return this._sandboxInfo; }
+
     const flavor = process.env.GRIST_SANDBOX_FLAVOR || 'unknown';
     const info = this._sandboxInfo = {
       flavor,
@@ -1408,6 +1409,8 @@ export class FlexServer implements GristServer {
       sandboxed: false,
       lastSuccessfulStep: 'none',
     } as SandboxInfo;
+    // Only meaningful on instances that handle documents.
+    if (!this._docManager) { return info; }
     try {
       const sandbox = createSandbox({
         server: this,
@@ -1432,10 +1435,7 @@ export class FlexServer implements GristServer {
     } catch (e) {
       info.error = String(e);
     }
-  }
-
-  public getSandboxInfo(): SandboxInfo|undefined {
-    return this._sandboxInfo;
+    return info;
   }
 
   public getInfo(key: string): any {
diff --git a/app/server/lib/GristServer.ts b/app/server/lib/GristServer.ts
index 602e7760..555d18ba 100644
--- a/app/server/lib/GristServer.ts
+++ b/app/server/lib/GristServer.ts
@@ -70,7 +70,7 @@ export interface GristServer {
   servesPlugins(): boolean;
   getBundledWidgets(): ICustomWidget[];
   getBootKey(): string|undefined;
-  getSandboxInfo(): SandboxInfo|undefined;
+  getSandboxInfo(): Promise<SandboxInfo>;
   getInfo(key: string): any;
   getJobs(): GristJobs;
 }
@@ -165,7 +165,7 @@ export function createDummyGristServer(): GristServer {
     getPlugins() { return []; },
     getBundledWidgets() { return []; },
     getBootKey() { return undefined; },
-    getSandboxInfo() { return undefined; },
+    getSandboxInfo() { throw new Error('no sandbox'); },
     getInfo(key: string) { return undefined; },
     getJobs(): GristJobs { throw new Error('no job system'); },
   };