Document the security model

Only admins can use the app, so who gets admin rights decides who can use
it. The database should run as a scoped user without superuser or
server file/program rights, since the SQL checks are an extra layer.
Explain why sensitive occ commands are not blocked: an admin can reach
the same information in other ways, so a blocklist would only give a
false sense of security and make the app less useful.

INSTALL-AIO.md

@@ -93,3 +93,6 @@ rather than reacting after the fact.
 
 Type `sql` in the OCCWeb terminal to run raw SQL directly against the Nextcloud
 database. Separate statements with `;`, use Shift+Enter for a new line and Enter to run.
+
+Before giving anyone access, read the Security section in the README: only
+admins can use the app, and the database user should have limited rights.

Makefile

@@ -134,14 +134,14 @@ check-app-name:
 		(echo "app_name is empty - is 'xpath' (libxml2-utils/perl-XML-XPath) installed?" >&2; exit 1)
 
 .PHONY: source
-source: check-app-name check-clean-tree
+source: check-app-name check-clean-tree check-on-branch
 	rm -rf $(source_build_directory)
 	mkdir -p $(source_build_directory)
 	git archive --format=tar --prefix=$(app_name)/ HEAD | gzip > $(source_package_name).tar.gz
 
 # Builds the source package for the app store, ignores php and js tests
 .PHONY: appstore
-appstore: check-app-name check-clean-tree
+appstore: check-app-name check-clean-tree check-on-branch
 	rm -rf $(appstore_build_directory)
 	mkdir -p $(appstore_build_directory)
 	git archive --format=tar --prefix=$(app_name)/ HEAD | gzip > $(appstore_package_name).tar.gz
@@ -178,8 +178,21 @@ check-clean-tree:
 	@git diff --quiet HEAD -- || \
 		(echo "Uncommitted changes to tracked files - commit before releasing (see 'git status')." >&2; exit 1)
 
+# `git archive HEAD` packages whatever commit the checkout currently points
+# to, regardless of branch - check-clean-tree only guards against *uncommitted*
+# changes, not against HEAD being the wrong branch entirely (a feature branch,
+# an old tag, a PR checkout, a detached HEAD). Override for the rare
+# intentional case (e.g. releasing from a branch on purpose):
+#   make appstore RELEASE_BRANCH=some-branch
+RELEASE_BRANCH ?= main
+
+.PHONY: check-on-branch
+check-on-branch:
+	@test "$$(git rev-parse --abbrev-ref HEAD)" = "$(RELEASE_BRANCH)" || \
+		(echo "HEAD is on '$$(git rev-parse --abbrev-ref HEAD)', not '$(RELEASE_BRANCH)' - pass RELEASE_BRANCH=<branch> to override if this is intentional." >&2; exit 1)
+
 .PHONY: version
-version: check-clean-tree
+version: check-clean-tree check-on-branch
 	@echo "Creating version v$(VERSION)"
 	@git tag v$(VERSION)
 	@git push $(RELEASE_REMOTE) v$(VERSION)

README.md

@@ -85,8 +85,36 @@ run the whole block in a single request — this matters for scripts like
 statement in one submission runs on the same database connection/session.
 Type `occ` to switch back to the normal occ-command mode.
 
-⚠️ There is no undo for `DELETE`/`UPDATE` statements — double check what
-you are about to run, ideally against a non-critical row/user first.
+Every statement that changes data or the schema (`DELETE`, `UPDATE`,
+`INSERT`, `DROP`, a data-modifying `WITH`, ...) asks for confirmation before
+the batch runs. There is still no undo — double check what you are about to
+run, ideally against a non-critical row/user first. Statements that access the
+server's filesystem or start programs are blocked.
+
+## Security
+
+OCC Web is a powerful tool: it gives whoever can open it the same control over
+Nextcloud as `occ` on the command line, plus direct access to the database.
+Treat access to it like shell access to the server.
+
+- **Only admins can use it.** Every endpoint checks that the user is a member
+  of the Nextcloud `admin` group, and the endpoints that run commands or SQL
+  also require a valid CSRF token; other users get HTTP 403, anonymous
+  requests HTTP 401. This is the security boundary of the app.
+- **Be careful who you make an admin.** Anyone in the `admin` group can use OCC
+  Web, and an admin can do everything this app does in other ways as well.
+  Deciding who gets admin rights is up to the instance operator, not this app.
+- **Run the database as a scoped user with limited rights.** The SQL mode runs
+  with the rights of Nextcloud's own database user. Make sure that user is not
+  a database superuser and cannot read or write files on the database server
+  or start programs there (on PostgreSQL: no superuser, no
+  `pg_execute_server_program`, `pg_read_server_files` or
+  `pg_write_server_files`). The checks in the SQL mode are an extra layer, not
+  a replacement for this.
+- **No blocklist of occ commands, on purpose.** We don't block commands that
+  show sensitive settings. It would make the app less useful, and an admin
+  can run such commands in a slightly more roundabout way anyway, so it would
+  only give a false sense of security.
 
 ## ⚠️ Warnings ⚠️
 

appinfo/info.xml

@@ -5,7 +5,7 @@
     <name>Extended OCC Web</name>
     <summary>OCC Commands in a web terminal</summary>
     <description><![CDATA[Run OCC Commands in a web terminal]]></description>
-    <version>0.4.3</version>
+    <version>0.4.5</version>
     <licence>agpl</licence>
     <author mail="dillard.blom@ensembia.com" >Dillard Blom</author>
     <namespace>OCCWeb</namespace>

js/index.js

@@ -14,6 +14,30 @@
     }
     var baseUrl = OC.generateUrl('/apps/extended_occweb');
 
+    // occ output is plain text with ANSI colors, but jQuery Terminal reads
+    // "[" and "]" as its own formatting syntax. Escape the brackets in the
+    // text (e.g. Symfony's "[alias]" in `occ list`) and leave the ANSI
+    // sequences alone, so from_ansi can still turn them into colors. This
+    // only works together with unixFormattingEscapeBrackets (see below).
+    //
+    // Output can contain text from users (display names, file names), so
+    // keep only color codes (SGR) and drop other escape sequences,
+    // backspaces and control characters, which could rewrite or hide text
+    // on screen. A lone CR becomes a newline.
+    function sanitizeOccOutput(output) {
+      return String(output)
+        .replace(/\x1B\][^\x07\x1B]*(\x07|\x1B\\)?/g, '')
+        .replace(/(\x1B\[[0-9;]*m)|\x1B\[[0-?]*[ -\/]*[@-~]|\x1B[@-_]?/g, function (match, sgr) {
+          return sgr || '';
+        })
+        .replace(/\r\n/g, '\n')
+        .replace(/\r/g, '\n')
+        .replace(/[\x00-\x08\x0B\x0C\x0E-\x1A\x1C-\x1F\x7F]/g, '')
+        .replace(/(\x1B\[[0-9;]*m)|[\[\]]/g, function (match, sgr) {
+          return sgr ? sgr : (match === '[' ? '[' : ']');
+        });
+    }
+
     // Current terminal mode: 'occ' (regular occ commands) or 'sql'
     // (running arbitrary SQL queries via /db/query).
     var mode = 'occ';
@@ -63,7 +87,8 @@
 
     // Quick client-side check for DELETE/UPDATE — purely for UX (to avoid
     // an extra round trip to the server). The backend always makes the
-    // final call (requiresConfirmation); this is just a hint.
+    // final call (requiresConfirmation) and also covers every other
+    // statement that changes something; this is just a hint.
     function scriptNeedsConfirmation(sql) {
       return splitStatements(sql).some(function (part) {
         var normalized = part.replace(/^(\s*--[^\n]*\n)*\s*/, '');
@@ -315,7 +340,8 @@
     }
 
     function askDeleteConfirmation(term, sql, message) {
-      var prompt = '[[;#ff5555;]' + (message || 'This script contains DELETE/UPDATE statement(s).') + ' Type "yes" to run it: ]';
+      var text = message || 'This script contains statement(s) that change data or the schema.';
+      var prompt = '[[;#ff5555;]' + $.terminal.escape_brackets(text) + ' Type "yes" to run it: ]';
       term.read(prompt).then(function (answer) {
         if ((answer || '').trim().toLowerCase() === 'yes') {
           sendSqlQuery(term, sql, true);
@@ -386,7 +412,7 @@
             headers: { requesttoken: OC.requestToken },
             data: JSON.stringify(occCommand)
           }).done(function (response) {
-            term.echo('\n' + response).resume();
+            term.echo('\n' + sanitizeOccOutput(response)).resume();
           }).fail(function (xhr, status) {
             term.echo('\n[[;#ff5555;]Request failed: ]' + $.terminal.escape_formatting(xhr.status + ' ' + xhr.statusText)).resume();
           });
@@ -398,6 +424,12 @@
         name: 'occ',
         prompt: OCC_PROMPT,
         completion: response,
+        // The overtyping and from_ansi formatters unescape brackets before
+        // parsing; this makes them escape the text again afterwards.
+        unixFormattingEscapeBrackets: true,
+        // Output can contain user-controlled text; don't turn URLs in it
+        // into links an admin might click by mistake.
+        convertLinks: false,
         keydown: function (e) {
           // Shift+Enter inserts a newline instead of running the command,
           // letting you type multi-line SQL scripts in sql mode.

lib/Controller/DbController.php

@@ -3,6 +3,7 @@
 namespace OCA\OCCWeb\Controller;
 
 use OC;
+use OCA\OCCWeb\Service\SqlGuard;
 use OCP\AppFramework\Controller;
 use OCP\IRequest;
 use OCP\IDBConnection;
@@ -19,26 +20,6 @@ class DbController extends Controller
     /** Maximum size of submitted SQL text in bytes (protection against OOM/DoS). */
     private const MAX_SQL_BYTES = 1048576; // 1 MB
 
-    /**
-     * Constructs that give access to the server's filesystem or allow
-     * launching external programs via SQL. These are blocked entirely,
-     * with no way to confirm past them — unlike DELETE, this isn't
-     * about data loss, it's about potential server takeover.
-     */
-    private const FORBIDDEN_PATTERNS = [
-        '/\bCOPY\b[\s\S]*\bPROGRAM\b/i' => 'COPY ... PROGRAM (runs external commands)',
-        '/\bCOPY\b[\s\S]*\b(FROM|TO)\b\s*\'/i' => "COPY ... FROM/TO 'file' (access to the server's filesystem)",
-        '/\bpg_read_binary_file\s*\(/i' => 'pg_read_binary_file()',
-        '/\bpg_read_file\s*\(/i' => 'pg_read_file()',
-        '/\bpg_ls_dir\s*\(/i' => 'pg_ls_dir()',
-        '/\bpg_stat_file\s*\(/i' => 'pg_stat_file()',
-        '/\blo_import\s*\(/i' => 'lo_import()',
-        '/\blo_export\s*\(/i' => 'lo_export()',
-        '/\bdblink(_connect)?\s*\(/i' => 'dblink() (connects to arbitrary databases)',
-        '/\bLOAD_FILE\s*\(/i' => 'LOAD_FILE()',
-        '/\bINTO\s+(OUTFILE|DUMPFILE)\b/i' => 'INTO OUTFILE/DUMPFILE',
-    ];
-
     private $db;
     private $groupManager;
     private $userSession;
@@ -60,106 +41,6 @@ class DbController extends Controller
         $this->logger = OC::$server->get(LoggerInterface::class);
     }
 
-    /**
-     * Strips leading single-line comments ("-- ...") before a query.
-     * After splitting a batch, such a comment can end up stuck to the
-     * next query and interfere with detecting its type (SELECT/SET/DELETE).
-     */
-    private function stripLeadingComments($query)
-    {
-        $query = ltrim($query);
-        while (preg_match('/^--[^\n]*\n/', $query)) {
-            $query = ltrim(preg_replace('/^--[^\n]*\n/', '', $query, 1));
-        }
-        return $query;
-    }
-
-    /**
-     * Strips ALL single-line (-- ...) and block-style C comments from a
-     * query, including ones sitting inside an expression (e.g. between a
-     * function name and its opening parenthesis — otherwise a forbidden
-     * construct could be hidden from findForbiddenConstruct that way).
-     * Used only for checking against forbidden constructs — the query
-     * that actually gets executed is left unmodified.
-     */
-    private function removeAllComments($query)
-    {
-        $query = preg_replace('/--[^\n]*/', '', $query);
-        $query = preg_replace('/\/\*[\s\S]*?\*\//', '', $query);
-        return $query;
-    }
-
-    /**
-     * Splits a batch of queries on ";" while respecting single and double
-     * quotes (strings and Postgres-escaped identifiers), so that a
-     * semicolon inside a 'string' or "identifier" (including '' / "" as
-     * an escaped quote) doesn't break up the split.
-     */
-    private function splitStatements($sql)
-    {
-        $statements = [];
-        $current = '';
-        $len = strlen($sql);
-        $quoteChar = null;
-
-        for ($i = 0; $i < $len; $i++) {
-            $ch = $sql[$i];
-
-            if ($quoteChar !== null) {
-                if ($ch === $quoteChar) {
-                    if ($i + 1 < $len && $sql[$i + 1] === $quoteChar) {
-                        $current .= $quoteChar . $quoteChar;
-                        $i++;
-                        continue;
-                    }
-                    $quoteChar = null;
-                }
-                $current .= $ch;
-                continue;
-            }
-
-            if ($ch === "'" || $ch === '"') {
-                $quoteChar = $ch;
-                $current .= $ch;
-                continue;
-            }
-
-            if ($ch === ';') {
-                $statements[] = trim($current);
-                $current = '';
-                continue;
-            }
-
-            $current .= $ch;
-        }
-
-        if (trim($current) !== '') {
-            $statements[] = trim($current);
-        }
-
-        return array_values(array_filter($statements, function ($s) {
-            return $s !== '';
-        }));
-    }
-
-    /**
-     * Returns a description of the forbidden construct found (filesystem
-     * access, running programs), or null if the query is safe in that
-     * regard. The comment-stripped version of the query is checked —
-     * otherwise the construct could be hidden by inserting a comment
-     * between the function name and "(".
-     */
-    private function findForbiddenConstruct($query)
-    {
-        $clean = $this->removeAllComments($query);
-        foreach (self::FORBIDDEN_PATTERNS as $pattern => $label) {
-            if (preg_match($pattern, $clean)) {
-                return $label;
-            }
-        }
-        return null;
-    }
-
     public function query()
     {
         // Deliberate defense-in-depth (see the equivalent check in
@@ -204,13 +85,13 @@ class DbController extends Controller
         // All queries run sequentially on the same DB connection (within a
         // single HTTP request), so SET retains its value for
         // current_setting() in the following queries of the same batch.
-        $queries = $this->splitStatements($sql);
+        $queries = SqlGuard::splitStatements($sql);
 
         // Access to the server's filesystem / running programs via SQL is
         // blocked entirely — this isn't about data loss (like DELETE), it's
         // about potential server takeover, and confirmation doesn't bypass it.
         foreach ($queries as $query) {
-            $forbidden = $this->findForbiddenConstruct($query);
+            $forbidden = SqlGuard::findForbiddenConstruct($query);
             if ($forbidden !== null) {
                 $this->logger->error('[extended_occweb] Blocked forbidden construct ({construct}) from {user}: {sql}', [
                     'app' => 'extended_occweb',
@@ -225,34 +106,29 @@ class DbController extends Controller
             }
         }
 
-        // DELETE and UPDATE are irreversible (or hard to reverse), so we
-        // require explicit confirmation from the client (confirm=true) before
-        // executing a single query from the batch.
-        $deleteCount = 0;
-        $updateCount = 0;
+        // Anything that isn't a plain read (DELETE, UPDATE, INSERT, DROP,
+        // a data-modifying CTE, ...) needs explicit confirmation from the
+        // client (confirm=true) before a single query of the batch runs.
+        $writeTypes = [];
         foreach ($queries as $query) {
-            $normalized = $this->stripLeadingComments($query);
-            if (stripos($normalized, 'DELETE') === 0) {
-                $deleteCount++;
-            } elseif (stripos($normalized, 'UPDATE') === 0) {
-                $updateCount++;
+            if (!SqlGuard::isReadOnly($query)) {
+                $type = SqlGuard::statementType($query) ?: 'UNKNOWN';
+                $writeTypes[$type] = ($writeTypes[$type] ?? 0) + 1;
             }
         }
 
-        if (($deleteCount > 0 || $updateCount > 0) && !$confirmed) {
+        if ($writeTypes !== [] && !$confirmed) {
             $parts = [];
-            if ($deleteCount > 0) {
-                $parts[] = "{$deleteCount} DELETE";
-            }
-            if ($updateCount > 0) {
-                $parts[] = "{$updateCount} UPDATE";
+            foreach ($writeTypes as $type => $count) {
+                $parts[] = "{$count} {$type}";
             }
             return new JSONResponse([
                 'success' => false,
                 'requiresConfirmation' => true,
-                'deleteCount' => $deleteCount,
-                'updateCount' => $updateCount,
-                'error' => 'Batch contains ' . implode(' and ', $parts) . ' statement(s) and was not executed. Resend with confirm=true to proceed.'
+                'deleteCount' => $writeTypes['DELETE'] ?? 0,
+                'updateCount' => $writeTypes['UPDATE'] ?? 0,
+                'writeCount' => array_sum($writeTypes),
+                'error' => 'Batch contains statement(s) that change data or the schema (' . implode(', ', $parts) . ') and was not executed. Resend with confirm=true to proceed.'
             ]);
         }
 
@@ -289,17 +165,16 @@ class DbController extends Controller
         }
 
         foreach ($queries as $query) {
-            $normalized = $this->stripLeadingComments($query);
-            $isSelect = stripos($normalized, 'SELECT') === 0;
-            $isSet = stripos($normalized, 'SET ') === 0;
-            $isDelete = stripos($normalized, 'DELETE') === 0;
-            $isUpdate = stripos($normalized, 'UPDATE') === 0;
+            $statementType = SqlGuard::statementType($query);
+            $isSet = $statementType === 'SET';
+            $isDelete = $statementType === 'DELETE';
+            $isUpdate = $statementType === 'UPDATE';
 
             try {
                 $stmt = $this->db->prepare($query);
                 $stmt->execute();
 
-                if ($isSelect) {
+                if ($statementType === 'SELECT' || ($statementType !== 'SET' && SqlGuard::isReadOnly($query))) {
                     // Read row by row and stop at MAX_ROWS, instead of
                     // fetchAll() + array_slice — otherwise a SELECT without
                     // LIMIT on a huge table would still pull everything into

lib/Controller/OccController.php

@@ -91,7 +91,7 @@ class OccController extends Controller
     try {
       $this->application->run($input, $this->output);
       return $this->output->fetch();
-    } catch (Exception $ex) {
+    } catch (\Throwable $ex) {
       $this->logger->error($ex->getMessage(), ['exception' => $ex]);
       return "error: " . $ex->getMessage();
     }

lib/Service/SqlGuard.php

@@ -0,0 +1,237 @@
+<?php
+
+namespace OCA\OCCWeb\Service;
+
+/**
+ * Checks run on SQL submitted through the SQL mode before it is executed.
+ *
+ * This is defense in depth, not a sandbox: dynamic SQL (EXECUTE with a
+ * built-up string, functions) can always get around a text-based check.
+ * The real protection is a database role without superuser rights,
+ * pg_execute_server_program or pg_read/write_server_files.
+ */
+class SqlGuard
+{
+    /**
+     * Constructs that give access to the server's filesystem or allow
+     * launching external programs via SQL. These are blocked entirely,
+     * with no way to confirm past them.
+     */
+    private const FORBIDDEN_PATTERNS = [
+        '/\bCOPY\b[\s\S]*\bPROGRAM\b/i' => 'COPY ... PROGRAM (runs external commands)',
+        // A COPY file name is a string literal: 'x', E'x', U&'x' or $$x$$
+        '/\bCOPY\b[\s\S]*\b(FROM|TO)\s+(E|U&)?(\'|\$)/i' => "COPY ... FROM/TO 'file' (access to the server's filesystem)",
+        '/\bpg_read_binary_file\s*\(/i' => 'pg_read_binary_file()',
+        '/\bpg_read_file\s*\(/i' => 'pg_read_file()',
+        '/\bpg_ls_dir\s*\(/i' => 'pg_ls_dir()',
+        '/\bpg_stat_file\s*\(/i' => 'pg_stat_file()',
+        '/\blo_import\s*\(/i' => 'lo_import()',
+        '/\blo_export\s*\(/i' => 'lo_export()',
+        '/\bdblink(_connect)?\s*\(/i' => 'dblink() (connects to arbitrary databases)',
+        '/\bLOAD_FILE\s*\(/i' => 'LOAD_FILE()',
+        '/\bINTO\s+(OUTFILE|DUMPFILE)\b/i' => 'INTO OUTFILE/DUMPFILE',
+    ];
+
+    /**
+     * Statements that start with one of these only read, unless
+     * WRITES_INSIDE_READ matches. SQLite's PRAGMA is left out on purpose:
+     * it can also change settings, so it asks for confirmation.
+     */
+    private const READ_ONLY_START = '/^(SELECT|WITH|SHOW|EXPLAIN|DESCRIBE|DESC|SET|VALUES|TABLE)\b/i';
+
+    /**
+     * What makes a statement that starts as a read change something:
+     * a data-modifying CTE (WITH x AS (DELETE ...)), SELECT ... INTO,
+     * SELECT ... FOR UPDATE, EXPLAIN ANALYZE (runs the statement),
+     * SET ROLE / SET SESSION AUTHORIZATION, MySQL's SET PASSWORD /
+     * GLOBAL / PERSIST, and functions with side
+     * effects. Statements that don't start as a read always need
+     * confirmation, so INSERT, DROP, CREATE etc. don't need to be here.
+     */
+    private const WRITES_INSIDE_READ = [
+        '/\b(INSERT|UPDATE|DELETE|MERGE|INTO|ANALYZE)\b/i',
+        '/^SET\s+(SESSION\s+|LOCAL\s+)?(ROLE|SESSION\s+AUTHORIZATION)\b/i',
+        '/^SET\s+(PASSWORD|GLOBAL|PERSIST|PERSIST_ONLY)\b|^SET\s+@@(GLOBAL|PERSIST|PERSIST_ONLY)\./i',
+        '/\b(pg_terminate_backend|pg_cancel_backend|pg_reload_conf|set_config|setval|nextval|lo_create|lo_put|lo_unlink|pg_switch_wal|pg_create_restore_point|pg_advisory_lock)\s*\(/i',
+    ];
+
+    /**
+     * Splits a batch on ";" outside of 'strings', "identifiers",
+     * $tag$dollar-quoted bodies$tag$ and comments, so a semicolon in a
+     * DO block or a quote in a comment doesn't break up the split.
+     *
+     * @return string[]
+     */
+    public static function splitStatements(string $sql): array
+    {
+        $statements = [];
+        $current = '';
+        $len = strlen($sql);
+        $i = 0;
+
+        while ($i < $len) {
+            $end = self::literalEnd($sql, $i) ?? self::commentEnd($sql, $i);
+            if ($end !== null) {
+                $current .= substr($sql, $i, $end - $i);
+                $i = $end;
+                continue;
+            }
+            if ($sql[$i] === ';') {
+                $statements[] = trim($current);
+                $current = '';
+            } else {
+                $current .= $sql[$i];
+            }
+            $i++;
+        }
+
+        $statements[] = trim($current);
+
+        return array_values(array_filter($statements, function ($s) {
+            return $s !== '';
+        }));
+    }
+
+    /**
+     * Replaces comments with a space, outside of literals only. A comment
+     * separates tokens, so dropping it without a space would glue
+     * "TO/ ** /PROGRAM" into one word; and a comment marker inside a
+     * literal ('/*') must not swallow the code after it.
+     */
+    public static function removeComments(string $query): string
+    {
+        return self::scan($query, false);
+    }
+
+    /**
+     * Also replaces string literals and quoted identifiers with a
+     * placeholder, so words inside them ('deleted', "update") don't count
+     * as keywords.
+     */
+    public static function removeLiterals(string $query): string
+    {
+        return self::scan($query, true);
+    }
+
+    private static function scan(string $sql, bool $replaceLiterals): string
+    {
+        $result = '';
+        $len = strlen($sql);
+        $i = 0;
+
+        while ($i < $len) {
+            $end = self::literalEnd($sql, $i);
+            if ($end !== null) {
+                $result .= $replaceLiterals ? ($sql[$i] === '"' ? '""' : "''") : substr($sql, $i, $end - $i);
+                $i = $end;
+                continue;
+            }
+            $end = self::commentEnd($sql, $i);
+            if ($end !== null) {
+                $result .= ' ';
+                $i = $end;
+                continue;
+            }
+            $result .= $sql[$i];
+            $i++;
+        }
+
+        return $result;
+    }
+
+    /**
+     * End offset of the 'string', E'string', "identifier" or $tag$body$tag$
+     * starting at $i, or null if none starts there. An unclosed literal
+     * runs to the end.
+     */
+    private static function literalEnd(string $sql, int $i): ?int
+    {
+        $len = strlen($sql);
+        $ch = $sql[$i];
+
+        if ($ch === "'" || $ch === '"') {
+            $backslashEscapes = $ch === "'" && $i > 0 && ($sql[$i - 1] === 'E' || $sql[$i - 1] === 'e')
+                && ($i === 1 || !preg_match('/[A-Za-z0-9_]/', $sql[$i - 2]));
+            $end = $i + 1;
+            while ($end < $len) {
+                if ($backslashEscapes && $sql[$end] === '\\') {
+                    $end += 2;
+                    continue;
+                }
+                if ($sql[$end] === $ch) {
+                    if ($end + 1 < $len && $sql[$end + 1] === $ch) {
+                        $end += 2;
+                        continue;
+                    }
+                    return $end + 1;
+                }
+                $end++;
+            }
+            return $len;
+        }
+
+        if ($ch === '$' && preg_match('/\G\$([A-Za-z_][A-Za-z0-9_]*)?\$/', $sql, $m, 0, $i)) {
+            $close = strpos($sql, $m[0], $i + strlen($m[0]));
+            return $close === false ? $len : $close + strlen($m[0]);
+        }
+
+        return null;
+    }
+
+    /**
+     * End offset of the -- or block comment starting at $i, or null.
+     */
+    private static function commentEnd(string $sql, int $i): ?int
+    {
+        if (substr($sql, $i, 2) === '--') {
+            $newline = strpos($sql, "\n", $i);
+            return $newline === false ? strlen($sql) : $newline;
+        }
+        if (substr($sql, $i, 2) === '/*') {
+            $close = strpos($sql, '*/', $i + 2);
+            return $close === false ? strlen($sql) : $close + 2;
+        }
+        return null;
+    }
+
+    /**
+     * Returns a description of the forbidden construct found, or null.
+     * Literals are kept here on purpose: a file name is a literal.
+     */
+    public static function findForbiddenConstruct(string $query): ?string
+    {
+        $clean = self::removeComments($query);
+        foreach (self::FORBIDDEN_PATTERNS as $pattern => $label) {
+            if (preg_match($pattern, $clean)) {
+                return $label;
+            }
+        }
+        return null;
+    }
+
+    /**
+     * The first keyword of a statement, upper case, e.g. SELECT or DELETE.
+     */
+    public static function statementType(string $query): string
+    {
+        $clean = ltrim(self::removeComments($query), " \t\n\r\0\x0B(");
+        return preg_match('/^([A-Za-z]+)/', $clean, $m) ? strtoupper($m[1]) : '';
+    }
+
+    /**
+     * Whether a statement only reads. Anything else needs confirmation.
+     */
+    public static function isReadOnly(string $query): bool
+    {
+        $clean = ltrim(self::removeLiterals(self::removeComments($query)), " \t\n\r\0\x0B(");
+        if (preg_match(self::READ_ONLY_START, $clean) !== 1) {
+            return false;
+        }
+        foreach (self::WRITES_INSIDE_READ as $pattern) {
+            if (preg_match($pattern, $clean) === 1) {
+                return false;
+            }
+        }
+        return true;
+    }
+}

tests/Unit/Service/SqlGuardTest.php

@@ -0,0 +1,172 @@
+<?php
+
+namespace OCA\OCCWeb\Tests\Unit\Service;
+
+use OCA\OCCWeb\Service\SqlGuard;
+use PHPUnit\Framework\TestCase;
+
+class SqlGuardTest extends TestCase
+{
+    public static function forbiddenProvider(): array
+    {
+        return [
+            ["COPY (SELECT 1) TO PROGRAM 'sh'"],
+            ["COPY (SELECT 'id') TO/**/PROGRAM 'sh'"],
+            ["COPY (SELECT 1) TO--x\nPROGRAM 'sh'"],
+            ["COPY (SELECT 1) TO '/tmp/x'"],
+            ["COPY (SELECT current_user) TO/**/'/tmp/x'"],
+            ["COPY (SELECT 1) TO E'/tmp/x'"],
+            ["COPY (SELECT 1) TO \$\$/tmp/x\$\$"],
+            ["COPY (SELECT 1) TO \$f\$/tmp/x\$f\$"],
+            ["COPY oc_users FROM '/etc/passwd'"],
+            ["SELECT pg_read_file/**/('/etc/passwd')"],
+            ["SELECT pg_ls_dir('/')"],
+            ["SELECT lo_import('/etc/passwd')"],
+            ["SELECT dblink_connect('host=x')"],
+            ["SELECT * FROM t INTO OUTFILE '/tmp/x'"],
+            ["COPY (SELECT '/*') TO PROGRAM 'sh' --*/"],
+            ["COPY (SELECT '--') TO PROGRAM 'sh'"],
+            ["COPY (SELECT \$\$/*\$\$) TO PROGRAM 'sh' --*/"],
+            ["COPY (SELECT \"/*\") TO PROGRAM 'sh' --*/"],
+            ["COPY (SELECT E'\\'/*') TO PROGRAM 'sh' --*/"],
+        ];
+    }
+
+    /**
+     * @dataProvider forbiddenProvider
+     */
+    public function testForbidden(string $sql): void
+    {
+        $this->assertNotNull(SqlGuard::findForbiddenConstruct($sql));
+    }
+
+    public static function allowedProvider(): array
+    {
+        return [
+            ['SELECT * FROM oc_users'],
+            ['COPY (SELECT 1) TO STDOUT'],
+        ];
+    }
+
+    /**
+     * @dataProvider allowedProvider
+     */
+    public function testAllowed(string $sql): void
+    {
+        $this->assertNull(SqlGuard::findForbiddenConstruct($sql));
+    }
+
+    public static function readOnlyProvider(): array
+    {
+        return [
+            ['SELECT * FROM oc_users'],
+            ['  -- comment' . "\n" . 'SELECT 1'],
+            ['/* comment */ SELECT 1'],
+            ['(SELECT 1) UNION (SELECT 2)'],
+            ['WITH x AS (SELECT 1) SELECT * FROM x'],
+            ["SELECT * FROM oc_jobs WHERE class = 'OCA\\\\Mail\\\\DeleteJob'"],
+            ["SELECT * FROM t WHERE status = 'deleted' OR note = 'update into'"],
+            ['SELECT "update" FROM t'],
+            ["SELECT replace(name, 'a', 'b') FROM t"],
+            ['SHOW search_path'],
+            ['EXPLAIN SELECT 1'],
+            ["SET myvar.uid = 'alice'"],
+            ['SET @uid = 5'],
+            ['SET SESSION sql_mode = \'\''],
+            ["SELECT \$\$DELETE FROM x\$\$"],
+            ["SELECT E'it\\'s delete' FROM t"],
+            ['VALUES (1)'],
+            ['DESCRIBE oc_users'],
+            ['DESC oc_users'],
+            ["SELECT '/* not a comment */' AS a, '-- nor this' AS b"],
+        ];
+    }
+
+    /**
+     * @dataProvider readOnlyProvider
+     */
+    public function testReadOnly(string $sql): void
+    {
+        $this->assertTrue(SqlGuard::isReadOnly($sql), $sql);
+    }
+
+    public static function writeProvider(): array
+    {
+        return [
+            ['DELETE FROM oc_preferences'],
+            ['/* not a leading -- comment */ DELETE FROM oc_preferences'],
+            ["-- x\nUPDATE t SET a = 1"],
+            ['WITH removed AS (DELETE FROM oc_preferences RETURNING *) SELECT count(*) FROM removed'],
+            ['WITH x AS (UPDATE t SET a = 1 RETURNING *) SELECT 1'],
+            ['INSERT INTO t VALUES (1)'],
+            ['TRUNCATE oc_preferences'],
+            ['DROP TABLE t'],
+            ['ALTER TABLE t ADD c int'],
+            ['CREATE TABLE t (a int)'],
+            ['GRANT ALL ON t TO x'],
+            ["DO \$\$BEGIN PERFORM 1; END\$\$"],
+            ['CALL p()'],
+            ['SELECT * INTO newtable FROM t'],
+            ['SELECT * FROM t FOR UPDATE'],
+            ['EXPLAIN ANALYZE DELETE FROM t'],
+            ['SET ROLE postgres'],
+            ['SET SESSION AUTHORIZATION postgres'],
+            ["SET PASSWORD = 'x'"],
+            ['SET GLOBAL max_connections = 10'],
+            ['SET PERSIST max_connections = 10'],
+            ['SET @@GLOBAL.max_connections = 10'],
+            ['SELECT pg_terminate_backend(123)'],
+            ["SELECT set_config('x', 'y', false)"],
+            ["SELECT setval('seq', 1)"],
+            ["SELECT nextval('seq')"],
+            ['MERGE INTO t USING s ON true WHEN MATCHED THEN DELETE'],
+            ['VACUUM'],
+            ['PRAGMA journal_mode = WAL'],
+            ["WITH a AS (SELECT '/*'), b AS (DELETE FROM t RETURNING 1) SELECT '*/'"],
+            ["SELECT '--' AS x FROM t WHERE a IN (SELECT 1) FOR UPDATE"],
+            ["WITH a AS (SELECT \$\$/*\$\$), b AS (DELETE FROM t RETURNING 1) SELECT '*/'"],
+            ['', ],
+        ];
+    }
+
+    /**
+     * @dataProvider writeProvider
+     */
+    public function testNeedsConfirmation(string $sql): void
+    {
+        $this->assertFalse(SqlGuard::isReadOnly($sql), $sql);
+    }
+
+    public function testStatementType(): void
+    {
+        $this->assertSame('DELETE', SqlGuard::statementType('/* x */ delete from t'));
+        $this->assertSame('WITH', SqlGuard::statementType("-- x\nWITH a AS (SELECT 1) SELECT 1"));
+        $this->assertSame('SELECT', SqlGuard::statementType('(SELECT 1)'));
+        $this->assertSame('', SqlGuard::statementType(''));
+    }
+
+    public static function splitProvider(): array
+    {
+        return [
+            ['SELECT 1; SELECT 2', ['SELECT 1', 'SELECT 2']],
+            ["SELECT ';'; SELECT 2", ["SELECT ';'", 'SELECT 2']],
+            ["SELECT 'it''s; x'", ["SELECT 'it''s; x'"]],
+            ['SELECT "a;b" FROM t', ['SELECT "a;b" FROM t']],
+            ["SELECT E'it\\'s; x'; SELECT 2", ["SELECT E'it\\'s; x'", 'SELECT 2']],
+            ["DO \$\$BEGIN PERFORM 1; PERFORM 2; END\$\$; SELECT 3", ["DO \$\$BEGIN PERFORM 1; PERFORM 2; END\$\$", 'SELECT 3']],
+            ["SELECT \$f\$a;\$\$;b\$f\$; SELECT 2", ["SELECT \$f\$a;\$\$;b\$f\$", 'SELECT 2']],
+            ["-- don't; split\nSELECT 1; SELECT 2", ["-- don't; split\nSELECT 1", 'SELECT 2']],
+            ["/* it's; here */ SELECT 1; SELECT 2", ["/* it's; here */ SELECT 1", 'SELECT 2']],
+            ['SELECT \$1; SELECT 2', ['SELECT \$1', 'SELECT 2']],
+            [' ; ;SELECT 1;', ['SELECT 1']],
+        ];
+    }
+
+    /**
+     * @dataProvider splitProvider
+     */
+    public function testSplitStatements(string $sql, array $expected): void
+    {
+        $this->assertSame($expected, SqlGuard::splitStatements($sql));
+    }
+}