Security audit.

This commit is contained in:
Scott Duensing 2026-06-24 20:22:21 -05:00
parent d3e763e7fc
commit aacb430d92
15 changed files with 136 additions and 28 deletions

View file

@ -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 };

View file

@ -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,

View file

@ -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 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;
};

View file

@ -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);

View file

@ -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`);

View file

@ -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" };
};

View file

@ -68,7 +68,9 @@ const escape = (s) => {
.replace(/&/g, "&amp;")
.replace(/</g, "&lt;")
.replace(/>/g, "&gt;")
.replace(/"/g, "&quot;");
.replace(/"/g, "&quot;")
.replace(/'/g, "&#39;")
.replace(/\//g, "&#x2F;");
};

View file

@ -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();
};

View file

@ -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 = () => {

View file

@ -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": {

View file

@ -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})</);
const secretMatch = addRes.body.match(/user-select-all[^>]*>([0-9a-f]{64})</);
if (!secretMatch) throw new Error("no shared secret in /peers/add response");
const secret = secretMatch[1];

View file

@ -105,9 +105,9 @@ const request = (t, method, path, opts) => {
const csrfOf = (h) => { const m = h.match(/name="_csrf" value="([^"]+)"/); return m ? m[1] : ""; };
const opCountOf = (h) => { const m = h.match(/<th>Ops recorded<\/th><td>(\d+)<\/td>/); return m ? parseInt(m[1], 10) : null; };
const opCountOf = (h) => { const m = h.match(/<td>Ops recorded<\/td><td>(\d+)<\/td>/); return m ? parseInt(m[1], 10) : null; };
const envIdOf = (h) => { const m = h.match(/env_id<\/strong> is <code>([0-9a-f-]{36})<\/code>/); return m ? m[1] : ""; };
const secretOf = (h) => { const m = h.match(/<p class="secret">([0-9a-f]+)<\/p>/); return m ? m[1] : ""; };
const secretOf = (h) => { const m = h.match(/user-select-all[^>]*>([0-9a-f]{64})</); return m ? m[1] : ""; };
const ddCsrf = async (t) => csrfOf((await request(t, "GET", "/admin/dev-deploy/peers")).body);

View file

@ -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 <code>([0-9a-f-]{36})<\/code>/); return m ? m[1] : ""; };
const opCountOf = (h) => { const m = h.match(/<th>Ops recorded<\/th><td>(\d+)<\/td>/); return m ? parseInt(m[1], 10) : null; };
const secretOf = (h) => { const m = h.match(/<p class="secret">([0-9a-f]+)<\/p>/); return m ? m[1] : ""; };
const opCountOf = (h) => { const m = h.match(/<td>Ops recorded<\/td><td>(\d+)<\/td>/); return m ? parseInt(m[1], 10) : null; };
const secretOf = (h) => { const m = h.match(/user-select-all[^>]*>([0-9a-f]{64})</); return m ? m[1] : ""; };
// Find the peers-table row carrying envId and pull its peer_id out of the row's
@ -210,9 +210,12 @@ const run = async () => {
// 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,

View file

@ -138,18 +138,18 @@ const bootstrapTenant = async (t) => {
// The dashboard renders the env_id in the "Env ID" row as:
// <tr><th>Env ID</th><td><code>UUID</code></td></tr>
// <tr><td>Env ID</td><td><code>UUID</code></td></tr>
// (see lib/routes.js dashboard()). Anchor on that row so a later <code> UUID
// cannot be mistaken for it.
const envIdOf = (html) => {
const m = html.match(/<th>Env ID<\/th><td><code>([0-9a-f-]{36})<\/code>/);
const m = html.match(/<td>Env ID<\/td><td><code>([0-9a-f-]{36})<\/code>/);
return m ? m[1] : "";
};
// The "Ops recorded" row is: <tr><th>Ops recorded</th><td>N</td></tr>
// The "Ops recorded" row is: <tr><td>Ops recorded</td><td>N</td></tr>
const opCountOf = (html) => {
const m = html.match(/<th>Ops recorded<\/th><td>(\d+)<\/td>/);
const m = html.match(/<td>Ops recorded<\/td><td>(\d+)<\/td>/);
return m ? parseInt(m[1], 10) : null;
};

View file

@ -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 <code>([0-9a-f-]{36})<\/code>/); return m ? m[1] : ""; };
const opCountOf = (h) => { const m = h.match(/<th>Ops recorded<\/th><td>(\d+)<\/td>/); return m ? parseInt(m[1], 10) : null; };
const secretOf = (h) => { const m = h.match(/<p class="secret">([0-9a-f]+)<\/p>/); return m ? m[1] : ""; };
const opCountOf = (h) => { const m = h.match(/<td>Ops recorded<\/td><td>(\d+)<\/td>/); return m ? parseInt(m[1], 10) : null; };
const secretOf = (h) => { const m = h.match(/user-select-all[^>]*>([0-9a-f]{64})</); return m ? m[1] : ""; };
// pull redirects 302 with ?msg=... (success) or ?err=... (failure); the message