diff --git a/create-a-container/bin/create-container.js b/create-a-container/bin/create-container.js index 766ed815..2ea89ea3 100755 --- a/create-a-container/bin/create-container.js +++ b/create-a-container/bin/create-container.js @@ -340,64 +340,23 @@ async function main() { console.log('Container configured'); } - // Apply environment variables and entrypoint - // First read defaults from the image, then merge with user-specified values - const defaultConfig = await client.lxcConfig(node.name, vmid); - const defaultEntrypoint = defaultConfig['entrypoint'] || null; - const defaultEnvStr = defaultConfig['env'] || null; - - // Parse default env vars - let mergedEnvVars = {}; - if (defaultEnvStr) { - const pairs = defaultEnvStr.split('\0'); - for (const pair of pairs) { - const eqIndex = pair.indexOf('='); - if (eqIndex > 0) { - mergedEnvVars[pair.substring(0, eqIndex)] = pair.substring(eqIndex + 1); - } - } - } - - // Merge user-specified env vars (user values override defaults) - const userEnvVars = container.environmentVars ? JSON.parse(container.environmentVars) : {}; - - // Load system-wide default env vars from Settings. - // Descriptions are metadata only and are not passed into the container. - let systemDefaultEnvVars = {}; - try { - const entries = await Setting.getDefaultContainerEnvVars(); - for (const entry of entries) { - if (entry.key && entry.key.trim()) { - systemDefaultEnvVars[entry.key.trim()] = entry.value || ''; - } - } - } catch (_) { - console.warn('Could not load default_container_env_vars from settings, skipping'); - } + // Snapshot the template's env/entrypoint onto the container record now, as + // if the user had supplied them (user-supplied values still win). Templates + // are mutable Docker refs we can't re-query on a later reconfigure, so we + // persist them here; otherwise a future reconfigure (which uses + // deleteMissing) would unset template-provided values that were never + // stored. System/NVIDIA defaults are intentionally left out — they stay + // configure-time-only. + const templateConfig = await client.lxcConfig(node.name, vmid); + await container.persistTemplateDefaults(templateConfig); - // Merge priority: image defaults < system defaults < per-container user values - mergedEnvVars = { ...mergedEnvVars, ...systemDefaultEnvVars, ...userEnvVars }; - - // Use user entrypoint if specified, otherwise keep default - const finalEntrypoint = container.entrypoint || defaultEntrypoint; - - // Build config to apply - const envConfig = {}; - if (finalEntrypoint) { - envConfig.entrypoint = finalEntrypoint; - } - if (Object.keys(mergedEnvVars).length > 0) { - envConfig.env = Object.entries(mergedEnvVars) - .map(([key, value]) => `${key}=${value}`) - .join('\0'); - } - + // Apply environment variables and entrypoint. Use the default + // (deleteMissing=false): only explicit values are pushed, nothing is unset. + // The record now already includes the template's values, and system/NVIDIA + // defaults are merged in by buildLxcEnvConfig. + const envConfig = await container.buildLxcEnvConfig(); if (Object.keys(envConfig).length > 0) { console.log('Applying environment variables and entrypoint...'); - if (defaultEntrypoint) console.log(`Default entrypoint: ${defaultEntrypoint}`); - if (defaultEnvStr) console.log(`Image default env vars: ${Object.keys(mergedEnvVars).length - Object.keys(userEnvVars).length - Object.keys(systemDefaultEnvVars).length}`); - if (Object.keys(systemDefaultEnvVars).length > 0) console.log(`System default env vars: ${Object.keys(systemDefaultEnvVars).length} from settings`); - if (Object.keys(userEnvVars).length > 0) console.log(`Per-container env vars: ${Object.keys(userEnvVars).length}`); await client.updateLxcConfig(node.name, vmid, envConfig); console.log('Environment/entrypoint configuration applied'); } @@ -447,11 +406,9 @@ async function main() { throw new Error('Could not extract MAC address from container configuration'); } - // Read back entrypoint and environment variables from config + // Read back configuration from Proxmox. console.log('Querying container configuration...'); const config = await client.lxcConfig(node.name, vmid); - const actualEntrypoint = config['entrypoint'] || null; - const actualEnv = config['env'] || null; // Read back the actual provisioned resources so downstream systems // (e.g. NetBox) mirror what the container really has rather than assuming @@ -459,28 +416,7 @@ async function main() { const actualCores = config['cores'] != null ? parseInt(config['cores'], 10) : null; const actualMemoryMb = config['memory'] != null ? parseInt(config['memory'], 10) : null; const actualDiskGb = parseRootfsSizeGb(config['rootfs']); - - // Parse NUL-separated env string back to JSON object - let environmentVars = {}; - if (actualEnv) { - const pairs = actualEnv.split('\0'); - for (const pair of pairs) { - const eqIndex = pair.indexOf('='); - if (eqIndex > 0) { - const key = pair.substring(0, eqIndex); - const value = pair.substring(eqIndex + 1); - environmentVars[key] = value; - } - } - } - - if (actualEntrypoint) { - console.log(`Entrypoint: ${actualEntrypoint}`); - } - if (Object.keys(environmentVars).length > 0) { - console.log(`Environment variables: ${Object.keys(environmentVars).length} vars`); - } - + // Get IP address from Proxmox interfaces API const ipv4Address = await client.getLxcIpAddress(node.name, vmid); @@ -493,8 +429,6 @@ async function main() { await container.update({ macAddress, ipv4Address, - entrypoint: actualEntrypoint, - environmentVars: JSON.stringify(environmentVars), status: 'running' }); diff --git a/create-a-container/bin/reconfigure-container.js b/create-a-container/bin/reconfigure-container.js index 5bcebf3a..ee5ba86e 100644 --- a/create-a-container/bin/reconfigure-container.js +++ b/create-a-container/bin/reconfigure-container.js @@ -80,8 +80,11 @@ async function main() { const client = await node.api(); console.log('Proxmox API client initialized'); - // Build config from environment variables and entrypoint - const lxcConfig = container.buildLxcEnvConfig(); + // Build config from environment variables and entrypoint. Pass + // deleteMissing so that clearing env vars or removing a custom entrypoint + // actually unsets them on the existing container (vs. create, which must + // preserve template-provided values). + const lxcConfig = await container.buildLxcEnvConfig({ deleteMissing: true }); if (Object.keys(lxcConfig).length > 0) { console.log('Applying LXC configuration...'); diff --git a/create-a-container/models/container.js b/create-a-container/models/container.js index e0ae3f18..721cebfe 100644 --- a/create-a-container/models/container.js +++ b/create-a-container/models/container.js @@ -21,41 +21,227 @@ module.exports = (sequelize, DataTypes) => { } /** - * Build LXC config object for environment variables and entrypoint - * Returns config suitable for Proxmox API updateLxcConfig - * @returns {object} Config object with 'env' and 'entrypoint' properties + * Normalize a set of environment variables into a safe, flat + * { KEY: stringValue } object suitable for building the Proxmox `env` + * string. This is the single place that decides what a valid env var is. + * + * Rules (applied to every source — user input, settings, image defaults): + * - Input that is not a plain object (e.g. an array or null) yields {}. + * - Keys are trimmed. A key is dropped unless it is a non-empty string with + * no whitespace and no `=` or NUL — i.e. it matches a conventional env + * var name. This prevents a key from corrupting the NUL-separated + * `KEY=value` encoding. + * - Values are coerced to strings. Entries whose value is null/undefined or + * a non-primitive (object/array) are dropped rather than stringified to + * something like "[object Object]". Values containing NUL are also + * dropped, since NUL is the pair separator. + * + * @param {*} input - Candidate env vars, ideally a { key: value } object + * @returns {object} Flat object of validated { KEY: stringValue } */ - buildLxcEnvConfig() { - const config = {}; - const deleteList = []; - - // Parse environment variables from JSON and format as NUL-separated list - // Format: KEY1=value1\0KEY2=value2\0KEY3=value3 - if (this.environmentVars) { - try { - const envObj = JSON.parse(this.environmentVars); - const envPairs = []; - for (const [key, value] of Object.entries(envObj)) { - if (key && value !== undefined) { - envPairs.push(`${key}=${value}`); - } - } - if (envPairs.length > 0) { - config['env'] = envPairs.join('\0'); - } else { - deleteList.push('env'); + static normalizeEnvVars(input) { + const out = {}; + if (!input || typeof input !== 'object' || Array.isArray(input)) return out; + + // Conventional env var name: starts with a letter or underscore, followed + // by letters, digits, or underscores. Excludes `=`, NUL, whitespace, etc. + const validKey = /^[A-Za-z_][A-Za-z0-9_]*$/; + + for (const [rawKey, rawValue] of Object.entries(input)) { + const key = typeof rawKey === 'string' ? rawKey.trim() : ''; + if (!validKey.test(key)) continue; + + // Only primitives (string/number/boolean) become values; skip + // null/undefined and objects/arrays. + if (rawValue === null || rawValue === undefined) continue; + if (typeof rawValue === 'object') continue; + const value = String(rawValue); + if (value.includes('\0')) continue; + + out[key] = value; + } + return out; + } + + /** + * Parse a Proxmox LXC `env` string (NUL-separated `KEY=value` pairs) into a + * normalized, flat { KEY: stringValue } object. Anything malformed is + * dropped via normalizeEnvVars. The inverse of the encoding done in + * buildLxcEnvConfig. + * @param {string|null|undefined} envStr - Raw Proxmox `env` value + * @returns {object} Flat object of validated { KEY: value } + */ + static parseLxcEnvString(envStr) { + if (!envStr || typeof envStr !== 'string') return {}; + const raw = {}; + for (const pair of envStr.split('\0')) { + const eq = pair.indexOf('='); + if (eq > 0) raw[pair.substring(0, eq)] = pair.substring(eq + 1); + } + return this.normalizeEnvVars(raw); + } + + /** + * Internal helper for buildLxcEnvConfig. + * Load the admin-defined system default environment variables from the + * Settings table, flattened to a validated { KEY: value } object. + * Descriptions are metadata only and are not included. Returns an empty + * object if the setting is missing or malformed. + * @returns {Promise} Flat object of { KEY: value } system defaults + */ + static async getSystemDefaultEnvVars() { + const Setting = this.sequelize.models.Setting; + const raw = {}; + try { + const entries = await Setting.getDefaultContainerEnvVars(); + for (const entry of entries) { + // getDefaultContainerEnvVars yields { key, value, description }. + if (entry && typeof entry.key === 'string') { + raw[entry.key] = entry.value; } - } catch (err) { - console.error('Failed to parse environment variables JSON:', err.message); } - } else { + } catch (_) { + console.warn('Could not load default_container_env_vars from settings, skipping'); + } + return this.normalizeEnvVars(raw); + } + + /** + * Internal helper for buildLxcEnvConfig. + * Parse the container's user-defined environment variables into a validated, + * flat { KEY: value } object. The database record only ever stores the + * variables the user explicitly provided — admin/system, image, and NVIDIA + * defaults are merged in at configure-time (see buildLxcEnvConfig) and are + * intentionally NOT persisted. + * @returns {object} Flat object of validated { KEY: value } user env vars + */ + parseEnvironmentVars() { + if (!this.environmentVars) return {}; + try { + return this.constructor.normalizeEnvVars(JSON.parse(this.environmentVars)); + } catch (err) { + console.error('Failed to parse environment variables JSON:', err.message); + return {}; + } + } + + /** + * Internal helper for buildLxcEnvConfig. + * Environment variables implied by this container's configuration that are + * applied as defaults but never stored in the DB record. Currently this is + * the NVIDIA GPU passthrough defaults, applied when nvidiaRequested is set. + * @returns {object} Flat object of { KEY: value } defaults + */ + nvidiaDefaultEnvVars() { + if (!this.nvidiaRequested) return {}; + return { + NVIDIA_VISIBLE_DEVICES: 'all', + NVIDIA_DRIVER_CAPABILITIES: 'utility compute' + }; + } + + /** + * Fold a template's environment variables and entrypoint into this + * container's persisted record, as if the user had supplied them. + * + * Called once at creation, after the template has been cloned, with the + * template's own LXC config. We can't recover these values later (templates + * are mutable Docker refs that may change or disappear before a reconfigure, + * and we don't cache them), so we snapshot them onto the record now. This + * keeps env/entrypoint stable across future reconfigures — which use + * deleteMissing and would otherwise unset template-provided values that were + * never stored. + * + * Precedence is template < user (user-supplied values win). System and + * NVIDIA defaults are intentionally NOT folded in here; they remain + * configure-time-only so they are not frozen into the record. + * + * Persists and returns nothing; mutates this instance. + * + * @param {object} templateConfig - The cloned template's LXC config + * @param {string} [templateConfig.env] - Proxmox NUL-separated `env` string + * @param {string} [templateConfig.entrypoint] - Template entrypoint + */ + async persistTemplateDefaults(templateConfig = {}) { + const templateEnv = this.constructor.parseLxcEnvString(templateConfig.env); + // template < user + const mergedEnv = { ...templateEnv, ...this.parseEnvironmentVars() }; + + const templateEntrypoint = + typeof templateConfig.entrypoint === 'string' && templateConfig.entrypoint.trim() + ? templateConfig.entrypoint.trim() + : null; + const userEntrypoint = this.entrypoint && this.entrypoint.trim() ? this.entrypoint.trim() : null; + const mergedEntrypoint = userEntrypoint || templateEntrypoint; + + await this.update({ + environmentVars: Object.keys(mergedEnv).length > 0 ? JSON.stringify(mergedEnv) : null, + entrypoint: mergedEntrypoint + }); + } + + /** + * Build the LXC config object for environment variables and entrypoint to + * deploy to Proxmox via updateLxcConfig. + * + * This is the single entrypoint for determining a container's effective + * environment. It owns the full merge of every env-var source, applied here + * at configure-time rather than being baked into the container's DB record: + * + * admin-defined system defaults < NVIDIA defaults < user-defined values + * + * (Image-provided defaults are submitted by the UI as user-defined values, + * so they arrive via parseEnvironmentVars and need no special handling.) + * + * Proxmox's config endpoint is a partial update: keys present in the body are + * set, omitted keys are left untouched, and a key is only removed if named in + * the special `delete` parameter. That distinction matters here: + * + * - On create, the container has just been cloned from a template that may + * carry its own `env`/`entrypoint`. We must NOT delete those when the user + * didn't provide a value, or we'd wipe the template's defaults. So the + * default (deleteMissing=false) simply omits anything with no value. + * - On reconfigure, the user may have cleared their last env var or removed a + * custom entrypoint, and that change must take effect — so the caller + * passes deleteMissing=true to emit `delete` for the now-empty fields. + * + * @param {object} [options] + * @param {boolean} [options.deleteMissing=false] - When true, env/entrypoint + * that resolve to empty are added to Proxmox's `delete` list (removing any + * existing value). When false, they are simply omitted, preserving whatever + * the container/template already has. + * @returns {Promise} Config object with 'env' and 'entrypoint' + * properties (and, when deleteMissing is set, a 'delete' list) + */ + async buildLxcEnvConfig({ deleteMissing = false } = {}) { + const config = {}; + const deleteList = []; + + // Merge precedence (lowest to highest): + // system defaults < NVIDIA defaults < user-defined values + // Every source is already normalized to a safe { KEY: stringValue } map + // (see normalizeEnvVars), so the encoding below cannot be corrupted. + const mergedEnvVars = { + ...(await this.constructor.getSystemDefaultEnvVars()), + ...this.nvidiaDefaultEnvVars(), + ...this.parseEnvironmentVars() + }; + + // Format as NUL-separated list: KEY1=value1\0KEY2=value2\0KEY3=value3 + const envPairs = []; + for (const [key, value] of Object.entries(mergedEnvVars)) { + envPairs.push(`${key}=${value}`); + } + if (envPairs.length > 0) { + config['env'] = envPairs.join('\0'); + } else if (deleteMissing) { deleteList.push('env'); } - + // Set entrypoint command if (this.entrypoint && this.entrypoint.trim()) { config['entrypoint'] = this.entrypoint.trim(); - } else { + } else if (deleteMissing) { deleteList.push('entrypoint'); } diff --git a/create-a-container/routers/api/v1/containers.js b/create-a-container/routers/api/v1/containers.js index 2ca64da8..70c2947d 100644 --- a/create-a-container/routers/api/v1/containers.js +++ b/create-a-container/routers/api/v1/containers.js @@ -27,6 +27,25 @@ const router = express.Router({ mergeParams: true }); router.use(apiAuth); +/** + * Convert the `environmentVars` request payload (an array of { key, value } + * objects) into the JSON string stored on the container record, or null when + * there are no valid vars. Keys/values are validated and normalized via + * Container.normalizeEnvVars so only safe env var names are persisted (keys + * containing `=`/NUL or non-primitive values are dropped at ingest time). + * @param {Array<{key: string, value: *}>} environmentVars + * @returns {string|null} + */ +function serializeUserEnvVars(environmentVars) { + if (!Array.isArray(environmentVars)) return null; + const flat = {}; + for (const e of environmentVars) { + if (e && typeof e.key === 'string') flat[e.key] = e.value; + } + const normalized = Container.normalizeEnvVars(flat); + return Object.keys(normalized).length > 0 ? JSON.stringify(normalized) : null; +} + function normalizeDockerRef(ref) { if (ref.startsWith('http://') || ref.startsWith('https://') || ref.startsWith('git@')) return ref; let tag = 'latest'; @@ -243,20 +262,9 @@ router.post( if (!hostname || !hostname.trim()) throw new ApiError(400, 'invalid_request', 'hostname is required'); const wantsNvidia = !!nvidiaRequested; - let envVarsJson = null; - if (Array.isArray(environmentVars) && environmentVars.length > 0) { - const envObj = {}; - for (const e of environmentVars) { - if (e?.key && e.key.trim()) envObj[e.key.trim()] = e.value || ''; - } - if (Object.keys(envObj).length > 0) envVarsJson = JSON.stringify(envObj); - } - if (wantsNvidia) { - const obj = envVarsJson ? JSON.parse(envVarsJson) : {}; - if (!obj.NVIDIA_VISIBLE_DEVICES) obj.NVIDIA_VISIBLE_DEVICES = 'all'; - if (!obj.NVIDIA_DRIVER_CAPABILITIES) obj.NVIDIA_DRIVER_CAPABILITIES = 'utility compute'; - envVarsJson = JSON.stringify(obj); - } + // Only the user-defined env vars are persisted on the container record; + // NVIDIA and admin-defined system defaults are merged in at configure-time. + const envVarsJson = serializeUserEnvVars(environmentVars); const imageRef = template === 'custom' ? customTemplate?.trim() : template; if (!imageRef) throw new ApiError(400, 'invalid_request', 'template is required'); @@ -389,11 +397,7 @@ router.put( let envVarsJson = container.environmentVars; if (!isRestartOnly && Array.isArray(environmentVars)) { - const obj = {}; - for (const e of environmentVars) { - if (e?.key) obj[e.key.trim()] = e.value || ''; - } - envVarsJson = Object.keys(obj).length > 0 ? JSON.stringify(obj) : null; + envVarsJson = serializeUserEnvVars(environmentVars); } else if (!isRestartOnly && !environmentVars) { envVarsJson = null; }