From 312d2331a871f8c572368c068a6770a409485556 Mon Sep 17 00:00:00 2001
From: Paul Fitzpatrick <paulfitz@alum.mit.edu>
Date: Tue, 22 Nov 2022 09:15:18 -0500
Subject: [PATCH] (core) lock down modification of the _grist_Attachments table

Summary:
Rows in the _grist_Attachments table have a special lifecycle,
being created by a special method, and deleted via a special
process. All other modifications are now rejected, for simplicity.

Test Plan: added test

Reviewers: dsagal, jarek

Reviewed By: dsagal, jarek

Differential Revision: https://phab.getgrist.com/D3712
---
 app/common/ActiveDocAPI.ts       |  1 +
 app/server/lib/ActiveDoc.ts      |  4 +++-
 app/server/lib/GranularAccess.ts | 19 +++++++++++++++++--
 sandbox/gvisor/run.py            |  7 ++++++-
 4 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/app/common/ActiveDocAPI.ts b/app/common/ActiveDocAPI.ts
index 181d1283..1cefa165 100644
--- a/app/common/ActiveDocAPI.ts
+++ b/app/common/ActiveDocAPI.ts
@@ -22,6 +22,7 @@ export interface ApplyUAExtendedOptions extends ApplyUAOptions {
   oldestSource?: number;  // If set, gives the timestamp of the oldest source the undo/redo
                           // action was built from, expressed as number of milliseconds
                           // elapsed since January 1, 1970 00:00:00 UTC
+  attachment?: boolean;   // If set, allow actions on attachments.
 }
 
 export interface ApplyUAResult {
diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts
index df9f7e03..2ee01b40 100644
--- a/app/server/lib/ActiveDoc.ts
+++ b/app/server/lib/ActiveDoc.ts
@@ -844,7 +844,9 @@ export class ActiveDoc extends EventEmitter {
       );
       const userActions: UserAction[] = await Promise.all(
         upload.files.map(file => this._prepAttachment(docSession, file)));
-      const result = await this.applyUserActions(docSession, userActions);
+      const result = await this._applyUserActionsWithExtendedOptions(docSession, userActions, {
+        attachment: true,
+      });
       this._updateAttachmentsSize().catch(e => {
         this._log.warn(docSession, 'failed to update attachments size', e);
       });
diff --git a/app/server/lib/GranularAccess.ts b/app/server/lib/GranularAccess.ts
index 25f6723e..3fa0fcb2 100644
--- a/app/server/lib/GranularAccess.ts
+++ b/app/server/lib/GranularAccess.ts
@@ -2346,9 +2346,24 @@ export class GranularAccess implements GranularAccessForBundle {
       return dummyAccessCheck;
     }
     const tableId = getTableId(a);
-    if (tableId.startsWith('_grist') && tableId !== '_grist_Attachments' && tableId !== '_grist_Cells') {
+    if (tableId.startsWith('_grist') && tableId !== '_grist_Cells') {
+      if (tableId === '_grist_Attachments') {
+        // If the back end is adding/removing an attachment, all
+        // necessary authentication has happened, and we can go ahead
+        // and do it. Perhaps the back end should just use an
+        // exceptional session for this, rather than a special
+        // flag. That would change attribution of the action in the
+        // log, so I stuck with a flag, but I'm not sure if
+        // attribution is particularly useful in this case.
+        if (this._activeBundle?.options?.attachment) {
+          return dummyAccessCheck;
+        }
+        // Users cannot take actions on _grist_Attachments through the regular
+        // action interface.
+        throw new Error('_grist_Attachments modification is not allowed');
+      }
       // Actions on any metadata table currently require the schemaEdit flag.
-      // Exception: the attachments table and cell info table, which needs to be reworked to be compatible
+      // Exception: the cell info table, which needs to be reworked to be compatible
       // with granular access.
 
       // Another exception: ensure owners always have full access to ACL tables, so they
diff --git a/sandbox/gvisor/run.py b/sandbox/gvisor/run.py
index d80274ac..1cf3c238 100755
--- a/sandbox/gvisor/run.py
+++ b/sandbox/gvisor/run.py
@@ -81,8 +81,13 @@ settings = {
   "ociVersion": "1.0.0",
   "process": {
     "terminal": include_bash,
+    # Match current user id, for convenience with mounts. For some versions of
+    # gvisor, default behavior may be better - if you see "access denied" problems
+    # during imports, try commenting this section out. We could make imports work
+    # for any version of gvisor by setting mode when using tmp.dir to allow
+    # others to list directory contents.
     "user": {
-      "uid": os.getuid(),  # match current user id, for convenience with mounts
+      "uid": os.getuid(),
       "gid": 0
     },
     "args": cmd_args,