More security issues addressed.

lib/apply.js

@@ -46,6 +46,8 @@ const {
     ensureManagedSchema,
     setRowUuid,
     findIdByRowUuid,
+    assertRowSyncAllowed,
+    assertManagedModeAllowed,
     COLUMN_NAME: ROW_UUID_COL
 } = require("./rowIdentity");
 const { portableToRow } = require("./rowPayload");
@@ -658,6 +660,7 @@ const applyInsertRow = async ({ op, payload }) => {
     if (!op.entity_uuid)                  throw new Error("insert_row missing row_uuid (entity_uuid)");
     const tbl = await findLocalTableByUuid(payload.table_uuid);
     if (!tbl) throw new Error(`local table for uuid=${payload.table_uuid} not found`);
+    await assertRowSyncAllowed(tbl, payload.table_uuid);
     await ensureManagedSchema(tbl.name);
     // Idempotency: if a row with this uuid already exists, skip
     const existing = await findIdByRowUuid(tbl.name, op.entity_uuid);
@@ -677,6 +680,7 @@ const applyUpdateRow = async ({ op, payload }) => {
     if (!op.entity_uuid)                  throw new Error("update_row missing row_uuid");
     const tbl = await findLocalTableByUuid(payload.table_uuid);
     if (!tbl) throw new Error(`local table for uuid=${payload.table_uuid} not found`);
+    await assertRowSyncAllowed(tbl, payload.table_uuid);
     await ensureManagedSchema(tbl.name);
     const localId = await findIdByRowUuid(tbl.name, op.entity_uuid);
     if (!localId) {
@@ -700,6 +704,7 @@ const applyDropRow = async ({ op, payload }) => {
     if (!op.entity_uuid)                  throw new Error("drop_row missing row_uuid");
     const tbl = await findLocalTableByUuid(payload.table_uuid);
     if (!tbl) return { status: "noop", reason: "table not present locally" };
+    await assertRowSyncAllowed(tbl, payload.table_uuid);
     const localId = await findIdByRowUuid(tbl.name, op.entity_uuid);
     if (!localId) return { status: "noop", reason: "row uuid not present locally" };
     await tbl.deleteRows({ id: localId });
@@ -712,6 +717,7 @@ const applySetTableMode = async ({ op, payload }) => {
     const tbl = await findLocalTableByUuid(payload.table_uuid);
     if (!tbl) throw new Error(`local table for uuid=${payload.table_uuid} not found`);
     const mode = payload.data_mode || "user";
+    assertManagedModeAllowed(tbl, mode);
     if (mode === "managed" || mode === "starter") {
         await ensureManagedSchema(tbl.name);
     }

lib/constants.js

@@ -1,7 +1,7 @@
 // Compile-time constants for the dev-deploy plugin.
 
 const PLUGIN_NAME    = "dev-deploy";
-const PLUGIN_VERSION = "0.0.6";
+const PLUGIN_VERSION = "0.0.7";
 
 // Namespace UUID for deterministic IDs derived from (kind, name).
 // Generated once via crypto.randomUUID() and frozen here forever.
@@ -29,6 +29,14 @@ const DATA_MODES = {
     USER:    "user"
 };
 
+// User tables that must NEVER receive peer-synced rows, and must never be flipped
+// into managed/starter mode, no matter what a peer sends. `users` is the crown
+// jewel: a synced row there is an injected account (role_id=1 => admin takeover).
+// The admin UI already refuses to manage `users` (routes.js); the apply AND revert
+// sides must refuse it too, since a malicious authenticated peer never goes through
+// the UI. One source of truth for every enforcement point.
+const ROW_SYNC_LOCKED_TABLES = ["users"];
+
 const DESTRUCTIVE_POLICY = {
     AUTO:    "auto",
     CONFIRM: "confirm",
@@ -72,6 +80,7 @@ module.exports = {
     OP_SCHEMA_VERSION,
     SYNCABLE_CONFIG_KEYS,
     DATA_MODES,
+    ROW_SYNC_LOCKED_TABLES,
     DESTRUCTIVE_POLICY,
     ENTITY_KINDS,
     fileLocationToId

lib/peerAuth.js

@@ -46,13 +46,16 @@ const readRawBody = async (req) => {
 
 // Record (env_id, nonce); returns false if it was already seen (replay) or on a
 // store error (fail closed -- better to reject than to allow a replay). Prunes
-// entries older than the skew window first: a timestamp that old is already
-// rejected by timestampWithinSkew, so its nonce can never be a valid replay.
+// entries older than 2x the skew window first. The acceptance window is 2x SKEW
+// wide (timestampWithinSkew accepts ts in [now-SKEW, now+SKEW]), and seen_ms is
+// the RECEIVE time which can trail ts by up to SKEW when a peer's clock leads, so
+// a nonce must be retained for 2x SKEW past receipt to cover the whole window a
+// captured request stays replayable -- pruning at 1x SKEW reopened that gap.
 const recordNonce = async (envId, nonce) => {
     const schema = db.getTenantSchemaPrefix();
     const now    = Date.now();
     try {
-        await db.query(`DELETE FROM ${schema}_dd_seen_nonces WHERE seen_ms < $1`, [now - SKEW_TOLERANCE_MS]);
+        await db.query(`DELETE FROM ${schema}_dd_seen_nonces WHERE seen_ms < $1`, [now - 2 * SKEW_TOLERANCE_MS]);
         await db.query(
             `INSERT INTO ${schema}_dd_seen_nonces (env_id, nonce, seen_ms) VALUES ($1, $2, $3)`,
             [envId, nonce, now]

lib/revert.js

@@ -43,13 +43,18 @@ const File            = require("@saltcorn/data/models/file");
 const db = require("@saltcorn/data/db");
 
 const { lookupByUuid }                    = require("./entityIds");
-const { ENTITY_KINDS, DATA_MODES }        = require("./constants");
+const { ENTITY_KINDS, DATA_MODES, SYNCABLE_CONFIG_KEYS } = require("./constants");
 const { refreshState }                    = require("./state");
 const { randomUuid }                      = require("./ids");
 const { enterOp }                         = require("./context");
 const { recordOpSafely }                  = require("./ops");
 const { portableToRow }                   = require("./rowPayload");
-const { ensureManagedSchema, findIdByRowUuid } = require("./rowIdentity");
+const {
+    ensureManagedSchema,
+    findIdByRowUuid,
+    assertRowSyncAllowed,
+    assertManagedModeAllowed
+} = require("./rowIdentity");
 
 
 const MODELS = {
@@ -252,6 +257,7 @@ const revertRow = async (action, op, payload) => {
     if (!tableUuid) return unsupported("row op missing table_uuid");
     const tbl = await findLocalTableByUuid(tableUuid);
     if (!tbl) return { status: "noop", reason: "table not present locally" };
+    await assertRowSyncAllowed(tbl, tableUuid);
     await ensureManagedSchema(tbl.name);
     const localId = await findIdByRowUuid(tbl.name, op.entity_uuid);
 
@@ -291,7 +297,10 @@ const revertTableMode = async (op, payload) => {
     if (!beforeMode) return unsupported("set_table_mode did not retain the prior mode");
     if (beforeMode === DATA_MODES.MANAGED || beforeMode === DATA_MODES.STARTER) {
         const tbl = await findLocalTableByUuid(tableUuid);
-        if (tbl) await ensureManagedSchema(tbl.name);
+        if (tbl) {
+            assertManagedModeAllowed(tbl, beforeMode);
+            await ensureManagedSchema(tbl.name);
+        }
     }
     const now = new Date().toISOString();
     const existing = await db.selectMaybeOne("_dd_table_modes", { table_uuid: tableUuid });
@@ -318,6 +327,13 @@ const revertTableMode = async (op, payload) => {
 const revertConfig = async (payload) => {
     if (!payload.key) return unsupported("set_config missing key");
     if (!("before" in payload)) return unsupported("set_config did not retain the prior value");
+    // Same allow-list applySetConfig enforces: a peer-supplied config op that was
+    // rejected on apply is still persisted (status=error) and shows a Revert button,
+    // so without this check reverting it would write arbitrary global config
+    // (disable_csrf_routes, HTML-bearing keys, SMTP) outside the sync boundary.
+    if (!SYNCABLE_CONFIG_KEYS.includes(payload.key)) {
+        return unsupported(`refusing to revert non-syncable config key "${payload.key}"`);
+    }
     const { getState } = require("@saltcorn/data/db/state");
     await getState().setConfig(payload.key, payload.before);
     return { status: "restored", key: payload.key };

lib/routes.js

@@ -24,6 +24,8 @@ const { signedFetch }                 = require("./transport");
 const { applyBatch, resolveConflict, resolveConflictByMerge, conflictFieldDiff } = require("./apply");
 const { revertOp }                    = require("./revert");
 const { DATA_MODES }                  = require("./constants");
+const { toAbsolutePath }              = require("./files");
+const File                            = require("@saltcorn/data/models/file");
 
 // Saltcorn native markup primitives -- the same ones core admin pages use, so
 // these pages inherit the active theme by construction (mkTable/renderForm/
@@ -963,9 +965,18 @@ const apiFile = async (req, res) => {
             res.status(404).json({ error: "file not found", uuid: uuid });
             return;
         }
-        const path  = require("path");
         const dbMod = require("@saltcorn/data/db");
-        const absPath = path.join(dbMod.connectObj.file_store, dbMod.getTenantSchema(), mapping.current_name);
+        // Route through the same containment helper the write path uses: assert the
+        // resolved path stays under this tenant's file_store. current_name is only
+        // ever a containment-checked relative_path today, but a future writer must
+        // not be able to turn this serve endpoint into an arbitrary-file read.
+        let absPath;
+        try {
+            absPath = toAbsolutePath(File, dbMod, mapping.current_name);
+        } catch (e) {
+            res.status(400).json({ error: "refusing file path outside tenant store" });
+            return;
+        }
         res.type("application/octet-stream");
         // dotfiles: 'allow' so paths containing .dev-state (etc.) aren't
         // silently treated as not-found by Express's default dotfile policy.

lib/rowIdentity.js

@@ -28,12 +28,44 @@ const db    = require("@saltcorn/data/db");
 const Table = require("@saltcorn/data/models/table");
 const Field = require("@saltcorn/data/models/field");
 
-const { runSuppressed } = require("./context");
+const { runSuppressed }                        = require("./context");
+const { DATA_MODES, ROW_SYNC_LOCKED_TABLES }   = require("./constants");
 
 
 const COLUMN_NAME = "_dd_row_uuid";
 
 
+// Gate a peer-supplied row op before it touches a table. A table may receive
+// synced rows ONLY when it is locally in managed/starter mode AND is not a locked
+// system table (see ROW_SYNC_LOCKED_TABLES). The producer/admin-UI side enforces
+// both (wrap.js mode-gating + the routes.js users lock), but the apply and revert
+// sides replay peer-supplied row payloads directly and must re-enforce it here --
+// otherwise a malicious authenticated peer could inject an admin row into `users`
+// (or write rows into any mapped table the admin never opted in). Throws so the
+// caller records the op as an error instead of applying it.
+const assertRowSyncAllowed = async (tbl, tableUuid) => {
+    if (ROW_SYNC_LOCKED_TABLES.includes(tbl.name)) {
+        throw new Error(`refusing row sync on locked table "${tbl.name}"`);
+    }
+    const modeRow = await db.selectMaybeOne("_dd_table_modes", { table_uuid: tableUuid });
+    const mode    = (modeRow && modeRow.data_mode) || DATA_MODES.USER;
+    if (mode !== DATA_MODES.MANAGED && mode !== DATA_MODES.STARTER) {
+        throw new Error(`refusing row sync on table "${tbl.name}": data_mode is "${mode}", not managed or starter`);
+    }
+};
+
+
+// Gate a set_table_mode op: a locked system table (e.g. users) must never be
+// flipped into managed/starter, which would otherwise open the row-sync path to
+// it. A legitimate managed table is never a locked one (the admin UI blocks it on
+// the source too), so this rejects only hostile/erroneous mode ops.
+const assertManagedModeAllowed = (tbl, mode) => {
+    if ((mode === DATA_MODES.MANAGED || mode === DATA_MODES.STARTER) && ROW_SYNC_LOCKED_TABLES.includes(tbl.name)) {
+        throw new Error(`refusing to set data_mode "${mode}" on locked table "${tbl.name}"`);
+    }
+};
+
+
 const tableSqlRef = (tableName) => {
     const schema = db.getTenantSchemaPrefix();
     return `${schema}"${db.sqlsanitize(tableName)}"`;
@@ -230,5 +262,7 @@ module.exports = {
     findIdByRowUuid,
     setRowUuid,
     newRowUuid,
-    allRowsWithUuid
+    allRowsWithUuid,
+    assertRowSyncAllowed,
+    assertManagedModeAllowed
 };

package.json

@@ -1,6 +1,6 @@
 {
   "name": "dev-deploy",
-  "version": "0.0.6",
+  "version": "0.0.7",
   "description": "Saltcorn plugin: migrate metadata changes (tables, fields, views, pages, triggers, roles, library, tags) across Dev/Test/Prod environments via an ops journal with stable UUIDs and HMAC-authenticated peer endpoints. Detects concurrent-edit conflicts and offers theirs/mine resolution. User-table row data is never touched.",
   "main": "index.js",
   "scripts": {