From 9d88fa5d65cf33b0d51417eb1170a19a9fc86104 Mon Sep 17 00:00:00 2001 From: salman Date: Sat, 26 Sep 2026 16:08:42 +0200 Subject: [PATCH] Harden the scan pipeline: input validation, RNG freeze, and re-entrancy guard - Expand From/Step/To scans count-based so descending (negative-step) scans work, reject zero/unreachable ranges, and cap points at 5000 - Give parse_formula an unbalanced-bracket guard instead of an infinite loop and use direct recursion - Return {ok:false,error} from CreateInpFile and validate projectile, layer composition, density, and thickness instead of the erroneous ErrMsg/$ErrMsg path - Freeze the RNG backend after prep_cfg refreshes All and pass it through writeAsciiFileAsync so scans never mix generators mid-flight - Wrap startSequence with a run-in-progress guard, disable the Run button, and snapshot runWorkPath for all output writes - Unify web/Electron readAsciiFileAsync to throw instead of returning 0 - Make checkDir synchronous (mkdirSync) and honor its return value - Fix setValues/template substitution parsing and write web input files byte-identical to Electron - Track deferred seed-validation and input-cap work in ToDo.txt --- ToDo.txt | 16 ++++ TrimSPelec.js | 18 ++-- TrimSPlib.js | 239 +++++++++++++++++++++++++++++++++++++++----------- TrimSPweb.js | 18 ++-- 4 files changed, 225 insertions(+), 66 deletions(-) diff --git a/ToDo.txt b/ToDo.txt index e69de29..e56a24e 100644 --- a/ToDo.txt +++ b/ToDo.txt @@ -0,0 +1,16 @@ +Seed handling and input hardening (deferred from the B-list fixes) +===================================================================== + +- Validate ranSeed in the GUI and startSequence: it must be an integer in a + safe range (e.g. -2^31 .. 2^31-1) if a fixed seed is requested. Blocks a + crafted share link from injecting a garbage seed value into the input deck. +- Derive per-scan-point seeds deterministically from the base seed and the + scan index, so a multi-point run is reproducible in any finish order + (currently: initialSeed + iScan, but only when the user set a seed). +- Warn when a shared configuration fixed the RanSeed: every scan point then + uses the same generator, which can produce statistically misleading results. +- Cap client-controlled magnitudes: numberProj (projectile count) and + valEnergy must be upper-bounded from share links before the engine runs, to + prevent an accidental or malicious "run forever / write enormous .rge" job. +- Consider persisting the effective per-point seed back into the *.seq output + metadata so results self-document their RNG state. \ No newline at end of file diff --git a/TrimSPelec.js b/TrimSPelec.js index 9a810dd..98ac8ed 100644 --- a/TrimSPelec.js +++ b/TrimSPelec.js @@ -94,14 +94,11 @@ function checkDir(directory) { console.log('Invalid directory passed to checkDir:', directory); return(0); } - if (!fs.existsSync(directory)) { - // Folder does not exist try to create it - fs.mkdir(directory,{recursive: true}, (err) => { - if (err) { - alert('Cannot create data folder '+directory); - return(0); - } - }); + try { + fs.mkdirSync(directory, { recursive: true }); + } catch (err) { + alert('Cannot create data folder '+directory); + return(0); } return(1); } @@ -148,7 +145,10 @@ async function readAsciiFileAsync(filename) { return await fs.promises.readFile(filename, 'utf8'); } catch(e) { alert('Failed to read '+filename); - return(0); + // Match the web backend contract: the scan pipeline awaits this and + // expects a rejected promise, not a falsy value that would later fail + // with a confusing "0.trim is not a function" error. + throw new Error('Failed to read '+filename); } } diff --git a/TrimSPlib.js b/TrimSPlib.js index 5708e09..0e8e4dd 100644 --- a/TrimSPlib.js +++ b/TrimSPlib.js @@ -7,6 +7,11 @@ const electronExecSync = (typeof module !== 'undefined' && module.exports) // This is an object that contains all GUI ids and values var All = new Object(); +// True while a simulation scan is running. Blocks re-entrant Run clicks that +// would otherwise reassign the global All and let two scans write the same +// files concurrently. +var runInProgress = false; + function isObject(item) { // simple object check @@ -3049,16 +3054,25 @@ function parse_formula (mf) { if(/^\d+/.test(mf)) throw new SyntaxError("Molecular formula should not start with a number!"); var mol = {}; // go into brackets and collect info + var bracketGuard = 0; while(mf.includes("(")){ + if (++bracketGuard > 100) { + throw new SyntaxError("Brackets are nested too deeply in this formula"); + } var x = mf.lastIndexOf("("); - var y = mf.indexOf(")", mf.lastIndexOf("(")); + var y = mf.indexOf(")", x); + if (y === -1) { + // An opening bracket without a matching closing one would otherwise + // rebuild the same string forever and freeze the tab. + throw new SyntaxError("Unbalanced parentheses in formula"); + } var z = mf.substr(x+1, y-x-1); var c = mf.substr(y+1); c = /^\d+/.exec(c); c=(c!= null)?Number(c[0]):1; // recursive - var a = this.parse_formula(z); + var a = parse_formula(z); var b = ""; for(var k in a){ b+=k+""+a[k]*c; @@ -3254,7 +3268,14 @@ function rho_fun() markDensityEstimated(rhoLi, false); } else { // suggest a material density based on composition (when it's undefined) - let layer_formula = parse_formula(chem); + let layer_formula; + try { + layer_formula = parse_formula(chem); + } catch (parseError) { + showTransientWarning("Could not parse composition: " + parseError.message, document.getElementById(rhoLi)); + document.getElementById(rhoLi).value = 0; + return; + } // determine the stoichiometry sum (for normalization) let stoichiometry_sum = 0; @@ -3434,18 +3455,15 @@ function updateScanSummary() var warning = ""; if (scanType === "scanLoop") { - var from = Number(document.getElementById("scanFrom").value); - var step = Number(document.getElementById("scanStep").value); - var to = Number(document.getElementById("scanTo").value); - if (!Number.isFinite(from) || !Number.isFinite(step) || !Number.isFinite(to)) { - warning = "Enter numeric From, Step, and To values."; - } else if (step <= 0) { - warning = "Step must be greater than zero."; - } else if (to < from) { - warning = "To must be greater than or equal to From."; + var range = rangeScanValues( + document.getElementById("scanFrom").value, + document.getElementById("scanStep").value, + document.getElementById("scanTo").value); + if (range.error) { + warning = range.error; } else { - pointCount = Math.floor((to - from) / step) + 1; - detail = `${from} to ${to}, step ${step}`; + pointCount = range.values.length; + detail = `${range.values[0]} to ${range.values[pointCount - 1]}, step ${document.getElementById("scanStep").value}`; } } else { var values = document.getElementById("scanList").value @@ -3595,16 +3613,21 @@ function setValues(content) { flines = lines.filter(function (str) {return PATTERN.test(str);}); for (var i=0; i All[key]); } else { - TemplateFile = TemplateFile.replace(key,All[key]); + TemplateFile = TemplateFile.replace(key, () => All[key]); } } } @@ -3743,6 +3776,39 @@ function yieldToRenderer() { // points in parallel. var TRIMSP_WEB_MAX_CONCURRENCY = 4; +// Upper bound on the number of points one scan may expand to. This guards +// against a crafted link or an accidental step that would otherwise generate +// an effectively unlimited list of jobs and freeze the tab. +var TRIMSP_MAX_SCAN_POINTS = 5000; + +// Expand a From/Step/To range into a list of scan values. Both ascending and +// descending steps are valid (e.g. scanning from high to low energy), so the +// old "loop while value <= to" form is replaced by a count-based expansion +// that avoids zero-step and floating-point drift. Returns {values} or {error}. +function rangeScanValues(fromText, stepText, toText) { + var from = Number(fromText); + var step = Number(stepText); + var to = Number(toText); + if (!Number.isFinite(from) || !Number.isFinite(step) || !Number.isFinite(to)) { + return { error: "Enter numeric From, Step, and To values." }; + } + if (step === 0) { + return { error: "Step must be non-zero." }; + } + if ((step > 0 && to < from) || (step < 0 && to > from)) { + return { error: "To must be reachable from From with the given Step." }; + } + var count = Math.floor(Math.abs((to - from) / step)) + 1; + if (count > TRIMSP_MAX_SCAN_POINTS) { + return { error: "Too many scan points (max " + TRIMSP_MAX_SCAN_POINTS + ")." }; + } + var values = []; + for (var i = 0; i < count; i++) { + values.push(Number((from + i * step).toPrecision(12))); + } + return { values: values }; +} + // Format the live profile legend from the scan parameter currently being run. function formatScanLegend(scanName, scanValue) { if (scanName === "E") { @@ -3814,7 +3880,7 @@ function renderCompletedScanResults(results, allValues) { return combinedSeqLines; } -async function startSequence() { +async function runSimulationSequence() { let cmd = ''; let trimBin = All['trimPath']+"/trimspNL"; let randStr = ''; @@ -3850,7 +3916,12 @@ async function startSequence() { updateStatusBar({ backend: 'Web backend' }); } updateStatusBar({ workPath: All["workPath"], run: 'Starting run...' }); - if (!await writeAsciiFileAsync(All["workPath"] + "/TrimSP.cfg", prep_cfg(0))) { + // prep_cfg() refreshes All from the DOM, so freeze the RNG backend now. + // The browser must ignore dropdown changes that happen while CGI requests + // are still in flight, otherwise one scan could mix generators and produce + // irreproducible results. + const rngType = All["rngType"] || "pcg32"; + if (!await writeAsciiFileAsync(All["workPath"] + "/TrimSP.cfg", prep_cfg(0), rngType)) { return(0); } @@ -3863,16 +3934,34 @@ async function startSequence() { var SValues = []; let ScanName = "E"; let ScanAttrib = "valEnergy"; + let scanError = ""; // Generate an arry of scanned values (one element for a single run) if (All["scanSeq"]) { // For multiple runs or a scan if (All["scanType"]=="scanVals") { - var SValues=All["scanList"].split(/,/); - var SdzValues=All["scanListdz"].split(/,/); + SValues = All["scanList"].split(/,/) + .map(function(v) {return v.trim();}) + .filter(function(v) {return v !== "";}) + .map(Number); + var SdzValues = All["scanListdz"].split(/,/) + .map(function(v) {return v.trim();}) + .filter(function(v) {return v !== "";}); if (SValues.length == SdzValues.length) {All["SdzFlag"]=1;} + if (SValues.length === 0) { + scanError = "Enter at least one scan value."; + } else if (SValues.some(function(v) {return !Number.isFinite(v);})) { + scanError = "Every scan value must be numeric."; + } else if (SValues.length > TRIMSP_MAX_SCAN_POINTS) { + scanError = "Too many scan values (max " + TRIMSP_MAX_SCAN_POINTS + ")."; + } } else { - for (let Val=parseInt(All["scanFrom"]);Val<=parseInt(All["scanTo"]);Val=Val+parseInt(All["scanStep"])) { - SValues.push(Val); + // rangeScanValues handles both ascending and descending steps and + // rejects zero-size or unreachable ranges before expanding. + var range = rangeScanValues(All["scanFrom"], All["scanStep"], All["scanTo"]); + if (range.error) { + scanError = range.error; + } else { + SValues = range.values; } } @@ -3895,7 +3984,15 @@ async function startSequence() { } } else { // For single run, array with single value of valEnergy - SValues.push(parseInt(All["valEnergy"])); + var singleEnergy = Number(All["valEnergy"]); + if (!Number.isFinite(singleEnergy)) { + scanError = "Energy must be numeric."; + } + SValues.push(singleEnergy); + } + if (scanError) { + updateStatusBar({ run: scanError }); + return(0); } // Freeze the initial GUI state and expand the scan into independent jobs. @@ -3914,8 +4011,23 @@ async function startSequence() { jobAll["ranSeed"] = String(initialSeed + iScan); } All = jobAll; - let inputFileContent = CreateInpFile(); - if (inputFileContent=="ERROR") { + let inputFileContent; + try { + inputFileContent = CreateInpFile(); + } catch (error) { + let message = (error && error.message) ? error.message : String(error); + updateStatusBar({ run: message }); + All = initialAll; + return(0); + } + if (inputFileContent && typeof inputFileContent === "object" + && inputFileContent.ok === false) { + updateStatusBar({ run: inputFileContent.error || "Could not build the input file." }); + All = initialAll; + return(0); + } + if (typeof inputFileContent !== "string" || inputFileContent.length === 0) { + updateStatusBar({ run: "Could not build the input file." }); All = initialAll; return(0); } @@ -3930,6 +4042,10 @@ async function startSequence() { } All = initialAll; + // Results must land in the work dir that existed when the run started, + // never in or next to whatever All points at if a second Run was attempted. + const runWorkPath = initialAll["workPath"]; + // Results are stored by original scan index. The plots and final merged // sequence file are always rebuilt from this ordered array. let results = new Array(jobs.length); @@ -3937,7 +4053,7 @@ async function startSequence() { // Read back the outputs of one completed run and convert them into the // normalized data structures used by the live plots. async function collectRunResult(job) { - let [cols,data]= await readDatFileAsync(All["workPath"]+"/"+job.filename+".rge"); + let [cols,data]= await readDatFileAsync(runWorkPath+"/"+job.filename+".rge"); let depth=data[0]; let nmuons=data[1]; let dz = (depth[1]-depth[0])/10; @@ -3946,7 +4062,7 @@ async function startSequence() { depth[i]=depth[i]/10; nmuons[i]=nmuons[i]/norm; } - let seqRunFile = All["workPath"]+"/"+job.filename+".seq"; + let seqRunFile = runWorkPath+"/"+job.filename+".seq"; let seqRunLines = (await readAsciiFileAsync(seqRunFile)).trim().split('\n'); return { index: job.index, @@ -3972,18 +4088,18 @@ async function startSequence() { // Web jobs are executed by the CGI as a side effect of writing the *.inp // file, so they only need the upload. Local jobs also need an explicit // engine launch, which the pool performs with executeFileAsync. - let rngType = initialAll["rngType"] || "pcg32"; const canRunPool = jobs.length > 1 && (webOrApp || typeof executeFileAsync === 'function'); if (!canRunPool) { // Sequential path for a single job, or the local fallback when no async // executor exists. Local single runs keep inherited stdio for the log. + try { for (const job of jobs) { - if (!await writeAsciiFileAsync(All["workPath"]+"/"+job.filename+".inp", job.inputFileContent)) { + if (!await writeAsciiFileAsync(runWorkPath+"/"+job.filename+".inp", job.inputFileContent, rngType)) { return(0); } if (!webOrApp) { - cmd = "cd " + All["workPath"]; + cmd = "cd " + runWorkPath; cmd += ";" + trimBin + ' ' + job.filename + ' ' + rngType; electronExecSync(cmd, {stdio: 'inherit'}); } @@ -3991,6 +4107,11 @@ async function startSequence() { updateStatusBar({ run: `Completed ${results.filter(Boolean).length}/${jobs.length}: ${job.legend}` }); await updateCompletedResults(); } + } catch (error) { + console.error('Scan job failed:', error); + updateStatusBar({ run: 'Run failed' }); + return(0); + } } else { // Bounded worker pool. Web jobs run server-side through the CGI; local jobs // use executeFileAsync. Jobs are inserted into the final result set by scan @@ -4021,11 +4142,11 @@ async function startSequence() { }); // Pass the selected RNG backend explicitly so sequential and // parallel runs use the same generator choice. - writeAsciiFileAsync(All["workPath"]+"/"+job.filename+".inp", job.inputFileContent) + writeAsciiFileAsync(runWorkPath+"/"+job.filename+".inp", job.inputFileContent, rngType) .then((written) => { if (!written) { throw new Error(`Could not submit ${job.filename}`); } if (webOrApp) { return; } - return executeFileAsync(trimBin, [job.filename, rngType], All["workPath"]); + return executeFileAsync(trimBin, [job.filename, rngType], runWorkPath); }) .then(() => collectRunResult(job)) .then(async (result) => { @@ -4056,9 +4177,9 @@ async function startSequence() { } let combinedSeqLines = buildCombinedSequenceLines(results); - let seq_file = All["workPath"]+"/"+All["fileNamePrefix"]+"_Seq_Results.dat"; + let seq_file = runWorkPath+"/"+initialAll["fileNamePrefix"]+"_Seq_Results.dat"; if (combinedSeqLines.length > 0) { - if (!await writeAsciiFileAsync(seq_file, combinedSeqLines.join('\n') + '\n')) { + if (!await writeAsciiFileAsync(seq_file, combinedSeqLines.join('\n') + '\n', rngType)) { return(0); } } @@ -4070,6 +4191,24 @@ async function startSequence() { return(0); } +async function startSequence() { + // Re-entrancy guard: starting a second run while one is in flight would + // reassign the global All and let two scans write the same server files. + if (runInProgress) { + updateStatusBar({ run: 'A run is already in progress.' }); + return(0); + } + runInProgress = true; + const runButton = document.getElementById("runSimulationButton"); + if (runButton) { runButton.disabled = true; } + try { + return await runSimulationSequence(); + } finally { + runInProgress = false; + if (runButton) { runButton.disabled = false; } + } +} + function readDatFile(filename) { // Read column data file and return // cols - labels of columns diff --git a/TrimSPweb.js b/TrimSPweb.js index 64204b8..d6e196b 100644 --- a/TrimSPweb.js +++ b/TrimSPweb.js @@ -155,22 +155,24 @@ function recordWebDiagnostic(filename, request, response) { // Async file IO through the CGI. Using fetch instead of a synchronous // XMLHttpRequest keeps the browser responsive while the server runs the // simulation, and lets independent scan points run concurrently. -async function writeAsciiFileAsync(filename, content) { +async function writeAsciiFileAsync(filename, content, rngType) { // Write string content into ascii file filename // Go via CGI script // Prepare CGI args use POST for long files let cgiargs = new URLSearchParams(); cgiargs.set("fn", filename); - // Forward the GUI-selected RNG backend to the CGI wrapper, which passes it - // to the TRIM.SP-NL engine as the optional second command-line argument. - let rngType = document.getElementById("rngType"); + // Forward the RNG backend that was frozen when the run started, so all + // points of one scan use the same generator even if the GUI dropdown is + // changed while CGI requests are still in flight. if (rngType) { - cgiargs.set("rngType", rngType.value); + cgiargs.set("rngType", rngType); } let lines = content.split(/\n/); let prefix = filename.split(/\//); for (let i=0; i