diff --git a/lib/apply.js b/lib/apply.js index 11f0023..735eeb4 100644 --- a/lib/apply.js +++ b/lib/apply.js @@ -35,7 +35,7 @@ const { constraintDisplayName } = require("./entityIds"); const { runSuppressed } = require("./context"); -const { ENTITY_KINDS, fileLocationToId } = require("./constants"); +const { ENTITY_KINDS, fileLocationToId, SYNCABLE_CONFIG_KEYS } = require("./constants"); const { refreshState } = require("./state"); const { sha256Buffer, writeFileBytes, toAbsolutePath } = require("./files"); const { signedFetchBinary } = require("./transport"); @@ -751,6 +751,12 @@ const applyUpdatePluginConfig = async ({ op, payload }) => { const applySetConfig = async ({ op, payload }) => { const { key, value } = payload || {}; if (!key) throw new Error("set_config missing key"); + // Refuse any key dev-deploy does not itself sync: a peer must not be able to + // write security-relevant config (disable_csrf_routes, custom HTML, SMTP) it + // was never meant to touch. Same allow-list the journalling side honors. + if (!SYNCABLE_CONFIG_KEYS.includes(key)) { + throw new Error(`set_config refused for non-syncable key "${key}"`); + } const { getState } = require("@saltcorn/data/db/state"); await getState().setConfig(key, value); return { status: "set", key: key }; diff --git a/lib/constants.js b/lib/constants.js index f9c8559..ba186e2 100644 --- a/lib/constants.js +++ b/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.4"; +const PLUGIN_VERSION = "0.0.6"; // Namespace UUID for deterministic IDs derived from (kind, name). // Generated once via crypto.randomUUID() and frozen here forever. @@ -11,6 +11,18 @@ const ID_NAMESPACE = "8b3a1e0d-4f6c-4d2a-9c5b-7e8f9a0b1c2d"; const OP_SCHEMA_VERSION = 1; +// The ONLY global _sc_config keys dev-deploy syncs. One source of truth for both +// the journalling side (wrap.js only records set_config ops for these keys) and +// the apply side (applySetConfig refuses any other key), so a compromised peer +// cannot write security-relevant config it was never meant to touch (e.g. +// disable_csrf_routes, custom HTML headers, SMTP creds). +const SYNCABLE_CONFIG_KEYS = [ + "menu_items", + "site_name", + "site_logo_id", + "base_url" +]; + const DATA_MODES = { MANAGED: "managed", STARTER: "starter", @@ -58,6 +70,7 @@ module.exports = { PLUGIN_VERSION, ID_NAMESPACE, OP_SCHEMA_VERSION, + SYNCABLE_CONFIG_KEYS, DATA_MODES, DESTRUCTIVE_POLICY, ENTITY_KINDS, diff --git a/lib/files.js b/lib/files.js index e02a110..4cbe15b 100644 --- a/lib/files.js +++ b/lib/files.js @@ -31,10 +31,19 @@ const toRelativePath = (File, absPath) => { // Convert a relative serve path back to the absolute path on this instance. +// SECURITY: relPath arrives from a (HMAC-authenticated) peer payload, so it is +// attacker-influenceable. Resolve it and assert it stays strictly within this +// tenant's file_store root -- otherwise a "../../.." (or absolute) path would let +// a peer write/read arbitrary files (e.g. /etc/cron.d, app JS) -> RCE. const toAbsolutePath = (File, db, relPath) => { - const path = require("path"); + const path = require("path"); const tenant = db.getTenantSchema(); - return path.join(db.connectObj.file_store, tenant, relPath); + const root = path.resolve(db.connectObj.file_store, tenant); + const abs = path.resolve(root, String(relPath || "")); + if (abs !== root && !abs.startsWith(root + path.sep)) { + throw new Error(`refusing file path outside tenant store: ${relPath}`); + } + return abs; }; diff --git a/lib/peerAuth.js b/lib/peerAuth.js index 9dbe9bb..4ad33b5 100644 --- a/lib/peerAuth.js +++ b/lib/peerAuth.js @@ -3,7 +3,8 @@ // Required headers: // X-DD-Env-Id caller's env UUID (looked up in _dd_peers) // X-DD-Timestamp ms-since-epoch; rejected if skew > 5 min -// X-DD-Nonce random per-request opaque bytes (replay padding) +// X-DD-Nonce random per-request opaque bytes; rejected if reused within +// the skew window (replay protection, see recordNonce) // X-DD-Signature hex HMAC-SHA256 over canonical string // // On success: req.dd_peer is set (peer row), req.body is the parsed JSON body, @@ -15,11 +16,13 @@ // read the exact raw bytes here and use them in the HMAC -- no JSON-canonical // fragility, no re-serialization assumptions about whitespace or key order. +const db = require("@saltcorn/data/db"); const { buildCanonical, normalizeHost, verifySignature, - timestampWithinSkew + timestampWithinSkew, + SKEW_TOLERANCE_MS } = require("./crypto"); const { findPeerByEnvId, @@ -41,6 +44,26 @@ 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. +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( + `INSERT INTO ${schema}_dd_seen_nonces (env_id, nonce, seen_ms) VALUES ($1, $2, $3)`, + [envId, nonce, now] + ); + return true; + } catch (e) { + return false; + } +}; + + const requirePeerAuth = async (req, res) => { for (const h of REQUIRED_HEADERS) { if (!req.headers[h]) { @@ -108,6 +131,14 @@ const requirePeerAuth = async (req, res) => { return null; } + // Replay guard: only after the signature is valid (so unauthenticated callers + // cannot flood the nonce store). A reused (env_id, nonce) within the skew + // window -- i.e. a captured request replayed verbatim -- is rejected here. + if (!(await recordNonce(callerEnvId, nonce))) { + res.status(401).json({ error: "nonce replay or store error" }); + return null; + } + if (bodyRaw) { try { req.body = JSON.parse(bodyRaw); diff --git a/lib/peers.js b/lib/peers.js index 0642461..b29da16 100644 --- a/lib/peers.js +++ b/lib/peers.js @@ -13,6 +13,27 @@ const { } = require("./crypto"); +// Validate an operator-entered peer base_url before we ever sign+send requests +// to it. Peers are deliberately allowed to be localhost / private addresses +// (Dev/Test/Prod often share a host or private network), so this is NOT IP +// filtering -- it just rejects unparseable URLs and non-http(s) schemes +// (file:, gopher:, data:, ...) that have no business being an HTTP peer. +const validatePeerUrl = (baseUrl) => { + let u; + try { + u = new URL(baseUrl); + } catch (e) { + throw new Error(`invalid base_url: ${baseUrl}`); + } + if (u.protocol !== "http:" && u.protocol !== "https:") { + throw new Error(`base_url must use http or https: ${baseUrl}`); + } + if (!u.host) { + throw new Error(`base_url has no host: ${baseUrl}`); + } +}; + + const rowToPeer = (row) => { if (!row) return null; return { @@ -79,6 +100,7 @@ const addPeer = async ({ envId, label, baseUrl, requireTls, existingSecret }) => if (!envId || !baseUrl) { throw new Error("addPeer requires envId and baseUrl"); } + validatePeerUrl(baseUrl); const dup = await findPeerByEnvId(envId); if (dup) { throw new Error(`peer with env_id ${envId} already exists`); diff --git a/lib/revert.js b/lib/revert.js index f2fc5a8..8aa7cf8 100644 --- a/lib/revert.js +++ b/lib/revert.js @@ -106,6 +106,15 @@ const revertCreate = async (kind, op) => { if (!inst) { return { status: "noop", reason: "entity no longer present" }; } + // Guard: reverting create_table DROPS the table. If it now holds user rows, + // refuse -- a metadata revert must never silently destroy app data. The admin + // can drop the table explicitly if that is genuinely intended. + if (kind === ENTITY_KINDS.TABLE && typeof inst.countRows === "function") { + const n = await inst.countRows({}); + if (n > 0) { + return unsupported(`refusing to revert create_table "${inst.name}": it holds ${n} row(s); drop it manually if intended`); + } + } await inst.delete(); return { status: "dropped" }; }; diff --git a/lib/routes.js b/lib/routes.js index ac26d2a..93fa7fd 100644 --- a/lib/routes.js +++ b/lib/routes.js @@ -68,7 +68,9 @@ const escape = (s) => { .replace(/&/g, "&") .replace(//g, ">") - .replace(/"/g, """); + .replace(/"/g, """) + .replace(/'/g, "'") + .replace(/\//g, "/"); }; diff --git a/lib/schema.js b/lib/schema.js index 071ab7e..c942e6c 100644 --- a/lib/schema.js +++ b/lib/schema.js @@ -199,6 +199,23 @@ const createDdAnchors = async () => { }; +// Replay-protection store: every accepted peer request's (env_id, nonce) is +// recorded here so a captured signed request cannot be replayed within the skew +// window. Raw + regenerable (losing it only reopens the small replay window). +const createDdSeenNonces = async () => { + const schema = db.getTenantSchemaPrefix(); + await db.query(` + CREATE TABLE IF NOT EXISTS ${schema}_dd_seen_nonces ( + env_id TEXT NOT NULL, + nonce TEXT NOT NULL, + seen_ms BIGINT NOT NULL, + PRIMARY KEY (env_id, nonce) + ) + `); + await db.query(`CREATE INDEX IF NOT EXISTS _dd_seen_nonces_seen ON ${schema}_dd_seen_nonces (seen_ms)`); +}; + + const createAllTables = async () => { // Identity -> _sc_config (migrate any legacy table first). await migrateEnvToConfig(); @@ -209,6 +226,7 @@ const createAllTables = async () => { await createDdEntityIds(); await createDdOps(); await createDdAnchors(); + await createDdSeenNonces(); }; diff --git a/lib/wrap.js b/lib/wrap.js index f97d77d..453f8e6 100644 --- a/lib/wrap.js +++ b/lib/wrap.js @@ -39,7 +39,7 @@ const { removeEntityRow, constraintDisplayName } = require("./entityIds"); -const { ENTITY_KINDS, fileLocationToId } = require("./constants"); +const { ENTITY_KINDS, fileLocationToId, SYNCABLE_CONFIG_KEYS } = require("./constants"); const { sha256File, toRelativePath } = require("./files"); const { toPlaceholders } = require("./payloadRefs"); const { @@ -797,12 +797,7 @@ const wrapPlugin = () => { // full key+value pair; apply replays it on the target via setConfig. // menu_items references pages/views by NAME (not id), which is naturally // stable across instances, so no UUID translation is needed. -const TRACKED_CONFIG_KEYS = new Set([ - "menu_items", - "site_name", - "site_logo_id", - "base_url" -]); +const TRACKED_CONFIG_KEYS = new Set(SYNCABLE_CONFIG_KEYS); const wrapSetConfig = () => { diff --git a/package.json b/package.json index a84327e..507064c 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "dev-deploy", - "version": "0.0.4", + "version": "0.0.6", "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": { diff --git a/test/e2e.js b/test/e2e.js index 0dceba8..c43463d 100644 --- a/test/e2e.js +++ b/test/e2e.js @@ -214,7 +214,7 @@ const main = async () => { const tables = sqlRows(MAIN.db, "SELECT name FROM sqlite_master WHERE type='table' AND name LIKE '_dd_%' ORDER BY name").join(","); // env identity moved to _sc_config; peers + table_modes are now registered // Saltcorn tables (still physical _dd_* tables, plus _sc_tables entries). - assert.equal(tables, "_dd_anchors,_dd_entity_ids,_dd_ops,_dd_peers,_dd_table_modes"); + assert.equal(tables, "_dd_anchors,_dd_entity_ids,_dd_ops,_dd_peers,_dd_seen_nonces,_dd_table_modes"); }); await test("_dd_ops has conflict_with_op_id column", async () => { const cols = sqlRows(MAIN.db, "PRAGMA table_info(_dd_ops)").map((r) => r.split("|")[1]); @@ -258,7 +258,7 @@ const main = async () => { await test("main /peers/add returns 200 with shared secret", async () => { assert.equal(addRes.status, 200); }); - const secretMatch = addRes.body.match(/class="secret">([0-9a-f]{64})]*>([0-9a-f]{64}) { const csrfOf = (h) => { const m = h.match(/name="_csrf" value="([^"]+)"/); return m ? m[1] : ""; }; -const opCountOf = (h) => { const m = h.match(/Ops recorded<\/th>(\d+)<\/td>/); return m ? parseInt(m[1], 10) : null; }; +const opCountOf = (h) => { const m = h.match(/Ops recorded<\/td>(\d+)<\/td>/); return m ? parseInt(m[1], 10) : null; }; const envIdOf = (h) => { const m = h.match(/env_id<\/strong> is ([0-9a-f-]{36})<\/code>/); return m ? m[1] : ""; }; -const secretOf = (h) => { const m = h.match(/

([0-9a-f]+)<\/p>/); return m ? m[1] : ""; }; +const secretOf = (h) => { const m = h.match(/user-select-all[^>]*>([0-9a-f]{64}) csrfOf((await request(t, "GET", "/admin/dev-deploy/peers")).body); diff --git a/test/mixedTopologyGate.js b/test/mixedTopologyGate.js index 88efad0..4bb90e4 100644 --- a/test/mixedTopologyGate.js +++ b/test/mixedTopologyGate.js @@ -109,8 +109,8 @@ const request = (inst, method, path, opts) => { const csrfOf = (h) => { const m = h.match(/name="_csrf" value="([^"]+)"/); return m ? m[1] : ""; }; const envIdOf = (h) => { const m = h.match(/env_id<\/strong> is ([0-9a-f-]{36})<\/code>/); return m ? m[1] : ""; }; -const opCountOf = (h) => { const m = h.match(/Ops recorded<\/th>(\d+)<\/td>/); return m ? parseInt(m[1], 10) : null; }; -const secretOf = (h) => { const m = h.match(/

([0-9a-f]+)<\/p>/); return m ? m[1] : ""; }; +const opCountOf = (h) => { const m = h.match(/Ops recorded<\/td>(\d+)<\/td>/); return m ? parseInt(m[1], 10) : null; }; +const secretOf = (h) => { const m = h.match(/user-select-all[^>]*>([0-9a-f]{64}) { // Build a peer-signed ingest exactly as transport.js would, then aim it at the // wrong tenant. Signature material is identical except the target host. const secretBuf = Buffer.from(secret, "hex"); + // Run-unique nonce (shared by both signed requests so they remain "identical + // except target host"), so the receiver's replay guard -- which records each + // accepted (env_id, nonce) -- does not reject the control on a rerun. + const nonce = require("crypto").randomBytes(16).toString("hex"); const mkHeaders = (targetHost, body) => { const ts = String(Date.now()); - const nonce = "00112233445566778899aabbccddeeff"; const canonical = buildCanonical({ timestamp: ts, nonce: nonce, diff --git a/test/mtGate.js b/test/mtGate.js index 741a2d3..09d71a0 100644 --- a/test/mtGate.js +++ b/test/mtGate.js @@ -138,18 +138,18 @@ const bootstrapTenant = async (t) => { // The dashboard renders the env_id in the "Env ID" row as: -// Env IDUUID +// Env IDUUID // (see lib/routes.js dashboard()). Anchor on that row so a later UUID // cannot be mistaken for it. const envIdOf = (html) => { - const m = html.match(/Env ID<\/th>([0-9a-f-]{36})<\/code>/); + const m = html.match(/Env ID<\/td>([0-9a-f-]{36})<\/code>/); return m ? m[1] : ""; }; -// The "Ops recorded" row is: Ops recordedN +// The "Ops recorded" row is: Ops recordedN const opCountOf = (html) => { - const m = html.match(/Ops recorded<\/th>(\d+)<\/td>/); + const m = html.match(/Ops recorded<\/td>(\d+)<\/td>/); return m ? parseInt(m[1], 10) : null; }; diff --git a/test/pullGate.js b/test/pullGate.js index b8b0f48..ef6f448 100644 --- a/test/pullGate.js +++ b/test/pullGate.js @@ -114,8 +114,8 @@ const request = (inst, method, path, opts) => { const csrfOf = (h) => { const m = h.match(/name="_csrf" value="([^"]+)"/); return m ? m[1] : ""; }; const envIdOf = (h) => { const m = h.match(/env_id<\/strong> is ([0-9a-f-]{36})<\/code>/); return m ? m[1] : ""; }; -const opCountOf = (h) => { const m = h.match(/Ops recorded<\/th>(\d+)<\/td>/); return m ? parseInt(m[1], 10) : null; }; -const secretOf = (h) => { const m = h.match(/

([0-9a-f]+)<\/p>/); return m ? m[1] : ""; }; +const opCountOf = (h) => { const m = h.match(/Ops recorded<\/td>(\d+)<\/td>/); return m ? parseInt(m[1], 10) : null; }; +const secretOf = (h) => { const m = h.match(/user-select-all[^>]*>([0-9a-f]{64})