From 9d88fa5d65cf33b0d51417eb1170a19a9fc86104 Mon Sep 17 00:00:00 2001 From: salman Date: Sat, 26 Sep 2026 16:08:42 +0200 Subject: [PATCH 1/9] 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 Date: Sat, 26 Sep 2026 16:32:38 +0200 Subject: [PATCH 2/9] Harden singleTrimSP.cgi: traversal fix, watchdog, pruner, archive lock, caps - P0 path-traversal fix: run-name class is dot-free with \z anchors, so '.', '..', and %0A-trailing values can never escape TRIMSP_TMPROOT. The deployed path allowlist is derived from the configured root instead of the old hardcoded /tmp prefix. - Replacing external with an alarm() watchdog that kills the engine's whole process group (setpgrp + kill KILL -pid), matching muSRFlow; describe_result maps -2/-1/signal to 124/127/128+sig. - prune_stale_jobs() ages out dirs (TrimSP.cfg marker only), orphaned .tgz, and leaked per-request logs; safe in shared /tmp (symlinks skipped, remove_tree safe mode, never touches non-trimsp dirs). - Archive is serialized per run with flock (.trimsp-lock); per-point engine runs stay concurrent. - Symlink/non-dir run dirs refused; mode 0700 so results are not world-readable. read/download also reject symlinked targets. - Request caps: MAX_REQUEST_LINES 20000, MAX_REQUEST_BYTES 4 MiB -> 413. - action=health deployment self-check; env overrides TRIMSP_BIN, TMPROOT, CMD_TIMEOUT, JOB_MAXAGE (sed anchor for the binary path kept intact). - Wire contract preserved: ok/phase/message/exitCode/archiveUrl/log JSON, text read, gzip download, real HTTP statuses. No TrimSPweb.js change. - New tests/cgi-tests.sh drives the real CGI offline (18 cases: traversal, %0A, escaping round-trip, success/fail/hang engine, 124 watchdog, archive+- download, pruner, symlink refusal, 413). deploy-web.sh now verifies the sed substitution embedded the selected engine path. --- ToDo.txt | 21 ++- scripts/deploy-web.sh | 8 + singleTrimSP.cgi | 398 +++++++++++++++++++++++++++++++++++++++--- tests/cgi-tests.sh | 324 ++++++++++++++++++++++++++++++++++ 4 files changed, 721 insertions(+), 30 deletions(-) create mode 100644 tests/cgi-tests.sh diff --git a/ToDo.txt b/ToDo.txt index e56a24e..6e0c6c2 100644 --- a/ToDo.txt +++ b/ToDo.txt @@ -13,4 +13,23 @@ Seed handling and input hardening (deferred from the B-list fixes) 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 + metadata so results self-document their RNG state. + +Deferred follow-ups from the C-list CGI hardening +================================================= + +- The perl "Statement unlikely to be reached" warning at the exec/_exit(127) + child branch of run_bounded_command is intentional (matching muSRFlow) and + does not affect `perl -c`; tests run with -X to keep output clean. +- No global concurrency cap is implemented (by design, mirroring muSRFlow): + concurrent per-point engine runs are allowed; archive is serialized per run + with flock. Revisit if a runaway share link floods scan points. +- Auth (MUCH-need, muSRFlow parity): none is implemented, keeping the read-only + /tmp contract. If this CGI is ever exposed beyond a trusted LAN, gate it with + httpd authentication or a shared-token header. +- Restart integration (retch): the watchdog returns exit code 124 and the JSON + "log" carries the description; a deployment that wants automated retries can + key off exitCode === 124 and re-post the same .inp. +- A future client change could surface TRIMSP_TMPROOT behind a shared + configuration; the browser path regex is derived from the same env var, so + both sides stay consistent. \ No newline at end of file diff --git a/scripts/deploy-web.sh b/scripts/deploy-web.sh index 9b03193..1556254 100755 --- a/scripts/deploy-web.sh +++ b/scripts/deploy-web.sh @@ -85,8 +85,16 @@ escaped_trimsp_binary="${escaped_trimsp_binary//|/\\|}" escaped_trimsp_binary="${escaped_trimsp_binary//\"/\\\"}" sed 's|^my \$trimsp_bin = .*|my $trimsp_bin = "'"$escaped_trimsp_binary"'";|' \ "$project_root/singleTrimSP.cgi" > "$cgi_stage" +perl -c "$cgi_stage" >/dev/null install -m 0755 "$cgi_stage" "$cgi_target" +# The installed CGI must embed the selected engine path exactly; a future +# source layout change could silently break the sed substitution above. +if ! grep -Fq 'my $trimsp_bin = "'"$trimsp_binary"'";' "$cgi_target"; then + printf 'Deployed CGI does not embed the selected engine path %s\n' "$trimsp_binary" >&2 + exit 1 +fi + # The browser assets and the CGI must be readable (and, for the CGI, # executable) by the web server. On the live server the web space is owned by # the apache user; on a developer machine the files may instead be owned by diff --git a/singleTrimSP.cgi b/singleTrimSP.cgi index eb6ee12..35bb3ca 100644 --- a/singleTrimSP.cgi +++ b/singleTrimSP.cgi @@ -2,25 +2,280 @@ # # Copyright Zaher Salman 2023-. # zaher.salman@psi.ch +# SPDX-License-Identifier: GPL-3.0-or-later +# +# singleTrimSP.cgi +# +# Web backend for TRIMSP-NL: accepts a run directory + file name with the +# submitted file content, runs the transport engine, and on request produces a +# downloadable results archive. +# +# Actions: +# (none) POST fn + rngType + lineN... write the file, then run/archive +# read GET fn stream a run-directory file +# download GET run stream the results archive (.tgz) +# health GET deployment self-check (JSON) +# +# The browser contract is deliberately unchanged from before the hardening: +# JSON responses keep ok/phase/message/exitCode/archiveUrl/log; +# read/download stream text/gzip with real HTTP status codes. +# +# Environment overrides (all optional; nothing here requires Apache config): +# TRIMSP_BIN - path to the trimspNL engine (the deploy script rewrites +# the $trimsp_bin literal below; env is a fallback) +# TRIMSP_TMPROOT - run root (default /tmp). The browser builds paths as +# //, so changing this requires the +# matching client change; the default is safe. +# TRIMSP_CMD_TIMEOUT - per-command watchdog seconds (default 300). Must stay +# above the engine's own internal bounds. +# TRIMSP_JOB_MAXAGE - stale run/archive lifetime seconds (default 86400). use strict; use warnings; use CGI; use File::Path qw(make_path remove_tree); use File::Temp qw(tempfile); +use Fcntl qw(:DEFAULT :flock); use JSON::PP qw(encode_json); # The web installer rewrites this path to the selected installed binary. my $trimsp_bin = "/usr/local/bin/trimspNL"; my $workroot = "/tmp"; +$trimsp_bin = $ENV{TRIMSP_BIN} if defined($ENV{TRIMSP_BIN}) && $ENV{TRIMSP_BIN} ne ""; +$workroot = $ENV{TRIMSP_TMPROOT} if defined($ENV{TRIMSP_TMPROOT}) && $ENV{TRIMSP_TMPROOT} ne ""; + +# Per-command watchdog. Every external command (trimspNL, tar) is bounded by an +# alarm() watchdog in run_bounded_command and killed as a process group, so a +# hung engine or tar cannot strand an Apache worker indefinitely. +my $CMD_TIMEOUT = (defined($ENV{TRIMSP_CMD_TIMEOUT}) && $ENV{TRIMSP_CMD_TIMEOUT} =~ /^\d+$/) + ? int($ENV{TRIMSP_CMD_TIMEOUT}) : 300; + +# Job/archive lifetime in seconds. prune_stale_jobs() removes trimsp run +# directories and orphaned archives older than this, bounding disk usage from +# long-running or abandoned browser sessions. +my $JOB_MAXAGE = (defined($ENV{TRIMSP_JOB_MAXAGE}) && $ENV{TRIMSP_JOB_MAXAGE} =~ /^\d+$/) + ? int($ENV{TRIMSP_JOB_MAXAGE}) : 86400; + +# Request-body caps. The browser posts one cfg, one .inp per scan point, and one +# sequence file; these limits stay far above real decks while stopping a crafted +# link from pushing unbounded content into the run directory. +my $MAX_REQUEST_LINES = 20000; +my $MAX_REQUEST_BYTES = 4 * 1024 * 1024; + +# Run name: alphanumerics, underscore, hyphen only (no dot, so ".", ".." can +# never be a path component). A run-name class that admitted dots is exactly +# the traversal hole muSRFlow carries; keep it dot-free. \z (not $) additionally +# rejects a trailing newline from a %0A tail. The file component stays nested in +# the same anchored pattern below, so no separate file-name regex is needed. +my $run_name_re = qr{^[A-Za-z0-9_-]+\z}; + +# Derive the path allowlist from the configured root so a TRIMSP_TMPROOT +# override stays consistent across every action. +my $work_path_re = qr{^\Q$workroot\E/([A-Za-z0-9_-]+)/([A-Za-z0-9_.-]+)\z}; + +# ---------------------------------------------------------------------------- +# Helpers +# ---------------------------------------------------------------------------- + +# Emit an error as a real HTTP status code with a JSON body, then exit. The +# payload carries the original ok/phase/message fields only, so error responses +# keep exactly the same shape the browser already understands. +sub json_error { + my ($status, $payload) = @_; + print "Status: $status\r\n"; + print "Content-Type: application/json; charset=UTF-8\r\n"; + print "Cache-Control: no-store\r\n\r\n"; + print encode_json($payload); + exit 0; +} + +# Shell quoting for browser-derived command components. After the path checks +# the tokens still go through shell_quote(); only server-side literals are ever +# interpolated unquoted. +sub shell_quote { + my ($s) = @_; + $s =~ s/'/'\\''/g; + return "'" . $s . "'"; +} + +# Run an arbitrary shell command with an alarm() watchdog, isolating the child +# in its own process group so a timeout can kill the whole pipeline (including +# grandchildren) without touching this CGI process. Returns system()'s waited +# status encoding, or -1 (could not start) / -2 (watchdog fired). +sub run_bounded_command { + my ($command) = @_; + my $pid = fork(); + return -1 if !defined($pid); + if ($pid == 0) { + setpgrp(0, 0); + umask(022); + open(STDIN, "<", "/dev/null") or _exit(127); + exec { "/bin/sh" } "/bin/sh", "-c", $command; + _exit(127); + } + + my $timed_out = 0; + local $SIG{ALRM} = sub { + $timed_out = 1; + kill "KILL", -$pid; + }; + my $status = -1; + eval { + alarm($CMD_TIMEOUT); + waitpid($pid, 0); + alarm(0); + $status = $?; + }; + my $wait_error = $@; + alarm(0); + if ($timed_out) { + # waitpid may be interrupted by SIGALRM or return after the killed child + # is reaped. A second call is harmless in the latter case and avoids a + # zombie in the former. + waitpid($pid, 0); + return -2; + } + if ($wait_error) { + kill "KILL", -$pid; + waitpid($pid, 0); + return -1; + } + return $status; +} + +# Convert a run_bounded_command result into (exit_code, human description) +# matching the vocabulary the browser shows in diagnostics. +sub describe_result { + my ($result) = @_; + if ($result == -2) { + return (124, "timed out after ${CMD_TIMEOUT}s"); + } elsif ($result == -1) { + return (127, "failed to start: $!"); + } elsif ($result & 127) { + return (128 + ($result & 127), "terminated by signal " . ($result & 127)); + } + return ($result >> 8, "exit code " . ($result >> 8)); +} + +# Accept or create the run directory. Refuses symlinks and non-directories so a +# pre-created /tmp/ link or planted file cannot redirect our writes, and +# uses mode 0700 so results are not world-readable in the shared temp root. +sub ensure_safe_work_dir { + my ($work_path) = @_; + if (-l $work_path || (-e $work_path && !-d $work_path)) { + return (0, "job directory is not a plain directory"); + } + if (!-d $work_path) { + if (!-d $workroot) { + return (0, "run root does not exist"); + } + my $ok = eval { + make_path($work_path, { mode => 0700, error => \my $err }); + !@{$err}; + }; + if (!$ok) { + return (0, "could not create job directory"); + } + } + return (1, ""); +} + +# Occupy the per-run archive lock. The archive step mutates the whole run +# directory (tar + remove_tree), so it is serialized; the per-point engine runs +# are intentionally concurrent and are not locked. The kernel releases flock +# when the CGI exits, so a dying worker cannot strand the lock. +sub lock_archive { + my ($work_path) = @_; + my $lock_path = "$work_path/.trimsp-lock"; + my $lfh; + if (!sysopen($lfh, $lock_path, O_RDWR | O_CREAT, 0600)) { + return undef; + } + if (!flock($lfh, LOCK_EX | LOCK_NB)) { + close($lfh); + return undef; + } + return $lfh; +} + +# Age out abandoned trimsp jobs. Mirrors muSRFlow's pruner but stays safe in +# the shared /tmp root: only directories that match the run-name rule AND +# contain a TrimSP.cfg marker are candidates, plus orphaned .tgz archives +# whose run directory is already gone, plus leaked per-request log files. +# Symlinks are skipped and remove_tree runs in safe mode, so a crafted entry is +# never followed or deleted. +sub prune_stale_jobs { + return 0 if !-d $workroot; + my $now = time(); + my $removed = 0; + + opendir(my $dh, $workroot) or return 0; + while (my $entry = readdir($dh)) { + next if $entry eq "." || $entry eq ".."; + next if $entry !~ /^[A-Za-z0-9_-]{5,}$/; + my $candidate = "$workroot/$entry"; + next if -l $candidate || !-d $candidate; + # Only ever touch directories this backend creates. + next if !-f "$candidate/TrimSP.cfg"; + my @stats = stat($candidate); + next if @stats < 9; + next if $stats[9] > $now - $JOB_MAXAGE; + # Never delete a directory that is currently being archived. + my $lock = "$candidate/.trimsp-lock"; + if (-e $lock) { + sysopen(my $lfh, $lock, O_RDWR | O_CREAT, 0600) or next; + if (!flock($lfh, LOCK_EX | LOCK_NB)) { close($lfh); next; } + close($lfh); + } + remove_tree($candidate, { safe => 1, error => \my $err }); + $removed++ if !@{$err}; + } + closedir($dh); + + # Orphaned archives: /tmp/.tgz whose run directory is already gone. + opendir($dh, $workroot) or return $removed; + while (my $entry = readdir($dh)) { + next if $entry !~ /^([A-Za-z0-9_-]{5,})\.tgz\z/; + next if -d "$workroot/$1"; + my $path = "$workroot/$entry"; + next if -l $path || !-f $path; + my @stats = stat($path); + next if @stats < 9 || $stats[9] > $now - $JOB_MAXAGE; + unlink($path); + $removed++; + } + closedir($dh); + + # Leaked per-request logs from requests that died before unlink(). + opendir($dh, $workroot) or return $removed; + while (my $entry = readdir($dh)) { + next if $entry !~ /^trimsp-([A-Za-z0-9_-]{5,})-[A-Za-z0-9_-]{4,}\z/; + my $path = "$workroot/$entry"; + next if -l $path || !-f $path; + my @stats = stat($path); + next if @stats < 9 || $stats[9] > $now - $JOB_MAXAGE; + unlink($path); + $removed++; + } + closedir($dh); + + return $removed; +} + +# ---------------------------------------------------------------------------- +# Request dispatch +# ---------------------------------------------------------------------------- + my $in = CGI->new; my $action = $in->param("action") // "run"; +# Opportunistically age out stale jobs before serving or mutating anything. +prune_stale_jobs(); + if ($action eq "read") { my $read_filename = $in->param("fn") // ""; - if ($read_filename !~ m{^/tmp/[A-Za-z0-9_-]+/[A-Za-z0-9_.-]+$} - || !-f $read_filename) { + if ($read_filename !~ $work_path_re || !-f $read_filename || -l $read_filename) { print "Status: 404 Not Found\r\n"; print "Content-Type: text/plain; charset=UTF-8\r\n\r\n"; print "Requested simulation output was not found.\n"; @@ -44,7 +299,7 @@ if ($action eq "read") { if ($action eq "download") { my $download_run = $in->param("run") // ""; my $archive_path = "$workroot/$download_run.tgz"; - if ($download_run !~ /^[A-Za-z0-9_-]+$/ || !-f $archive_path) { + if ($download_run !~ $run_name_re || !-f $archive_path || -l $archive_path) { print "Status: 404 Not Found\r\n"; print "Content-Type: text/plain; charset=UTF-8\r\n\r\n"; print "Requested results archive was not found.\n"; @@ -69,20 +324,48 @@ if ($action eq "download") { exit 0; } +if ($action eq "health") { + my %probe; + $probe{engine} = { + exists => (-e $trimsp_bin) ? JSON::PP::true : JSON::PP::false, + executable => (-x $trimsp_bin) ? JSON::PP::true : JSON::PP::false, + size => (-f $trimsp_bin && !-l $trimsp_bin) ? (-s $trimsp_bin || 0) : 0, + }; + $probe{runRoot} = { + exists => (-d $workroot) ? JSON::PP::true : JSON::PP::false, + writable => (-w $workroot) ? JSON::PP::true : JSON::PP::false, + }; + $probe{settings} = { + commandTimeoutSeconds => $CMD_TIMEOUT, + jobMaxAgeSeconds => $JOB_MAXAGE, + maxRequestBytes => $MAX_REQUEST_BYTES, + maxRequestLines => $MAX_REQUEST_LINES, + }; + print "Content-Type: application/json\r\n"; + print "Cache-Control: no-store\r\n\r\n"; + print encode_json({ + ok => JSON::PP::true, + action => "health", + probe => \%probe, + }); + exit 0; +} + +# --------------------------------------------------------------------------- +# Write + run path (default action). The browser posts fn + rngType + lineN... +# --------------------------------------------------------------------------- + my $requested_filename = $in->param("fn") // ""; my $rng_type = $in->param("rngType") // "pcg32"; # Accept only the private run path produced by TrimSPweb.js. Restricting both # path components prevents CGI parameters from selecting arbitrary files. -if ($requested_filename !~ m{^/tmp/([A-Za-z0-9_-]+)/([A-Za-z0-9_.-]+)$}) { - print "Status: 400 Bad Request\r\n"; - print "Content-Type: application/json\r\n\r\n"; - print encode_json({ +if ($requested_filename !~ $work_path_re) { + json_error(400, { ok => JSON::PP::false, phase => "request", message => "Invalid simulation filename", }); - exit 0; } my ($run_name, $file_name) = ($1, $2); @@ -98,18 +381,36 @@ $run_base =~ s/\.inp$//; my @line_parameters = sort { ($a =~ /line(\d+)/)[0] <=> ($b =~ /line(\d+)/)[0] } grep { /^line\d+$/ } $in->param; +if (@line_parameters > $MAX_REQUEST_LINES) { + json_error(413, { + ok => JSON::PP::false, + phase => "request", + message => "Too many submitted lines", + }); +} my $content = join("\n", map { scalar($in->param($_) // "") } @line_parameters) . "\n"; +if (length($content) > $MAX_REQUEST_BYTES) { + json_error(413, { + ok => JSON::PP::false, + phase => "request", + message => "Submitted file content is too large", + }); +} -make_path($work_path, { mode => 0755 }) unless -d $work_path; +my ($dir_ok, $dir_message) = ensure_safe_work_dir($work_path); +if (!$dir_ok) { + json_error(500, { + ok => JSON::PP::false, + phase => "request", + message => $dir_message, + }); +} open(my $output_file, ">", $filename) or do { - print "Status: 500 Internal Server Error\r\n"; - print "Content-Type: application/json\r\n\r\n"; - print encode_json({ + json_error(500, { ok => JSON::PP::false, phase => "write", message => "Could not write the submitted simulation file", }); - exit 0; }; print {$output_file} $content; close($output_file); @@ -122,6 +423,16 @@ my $log_content = ""; my $request_logfile; my $ok = JSON::PP::true; +# Append diagnostics to the private request log before and after a command so +# the JSON log field explains timeouts/signals instead of a bare exit code. +sub append_log { + my ($logfile, $text) = @_; + return unless $logfile; + return unless open(my $lfh, ">>", $logfile); + print {$lfh} $text; + close($lfh); +} + if ($filename =~ /_Seq_Results\.dat$/) { $phase = "archive"; unlink("$work_path/edist"); @@ -132,16 +443,40 @@ if ($filename =~ /_Seq_Results\.dat$/) { my ($log_handle, $logfile) = tempfile("trimsp-$run_name-XXXXXX", DIR => $workroot, UNLINK => 0); close($log_handle); $request_logfile = $logfile; - my $command = "tar -czf '$archive_path' -C '$work_path' . > '$logfile' 2>&1"; - system($command); - $exit_code = $? == -1 ? 127 : $? >> 8; - if ($exit_code == 0) { - $archive_url = "/cgi-bin/singleTrimSP.cgi?action=download&run=$run_name"; - $message = "Created results archive"; - remove_tree($work_path); - } else { + + my $lock_fh = lock_archive($work_path); + if (!$lock_fh) { $ok = JSON::PP::false; - $message = "Could not create results archive"; + $message = "Another archive operation is active in this run"; + } else { + # Replace a previous archive defensively, refusing a symlink target. + if (-e $archive_path || -l $archive_path) { + if (-l $archive_path || !-f $archive_path || !unlink($archive_path)) { + $ok = JSON::PP::false; + $message = "Could not replace the previous results archive"; + } + } + if ($ok) { + my $command = "tar -czf " . shell_quote($archive_path) + . " -C " . shell_quote($work_path) + . " . >> " . shell_quote($logfile) . " 2>&1"; + append_log($logfile, "\n[archive] \$ $command\n"); + my $result = run_bounded_command($command); + ($exit_code, my $completion) = describe_result($result); + append_log($logfile, "[${completion}]\n"); + if ($exit_code == 0 && -s $archive_path) { + $archive_url = "/cgi-bin/singleTrimSP.cgi?action=download&run=$run_name"; + $message = "Created results archive"; + remove_tree($work_path); + } else { + # A run that produced correct physics must not be reported as a + # failure just because archiving failed. + $ok = JSON::PP::false; + $message = "Could not create results archive"; + unlink($archive_path) if -f $archive_path && !-l $archive_path; + } + } + close($lock_fh); } } elsif ($filename !~ /TrimSP\.cfg$/) { $phase = "simulation"; @@ -151,14 +486,18 @@ if ($filename =~ /_Seq_Results\.dat$/) { # All user-derived command components were restricted above. Each request # receives a private log file so concurrent users never share output. - my $command = "cd '$work_path' && timeout 300 '$trimsp_bin' " - . "'$run_base' '$rng_type' >> '$logfile' 2>&1"; - system($command); - $exit_code = $? == -1 ? 127 : $? >> 8; + my $command = "cd " . shell_quote($work_path) . " && " + . shell_quote($trimsp_bin) . " " + . shell_quote($run_base) . " " + . shell_quote($rng_type) . " >> " . shell_quote($logfile) . " 2>&1"; + append_log($logfile, "\n[simulation] \$ $command\n"); + my $result = run_bounded_command($command); + ($exit_code, my $completion) = describe_result($result); + append_log($logfile, "[${completion}]\n"); $ok = JSON::PP::false if $exit_code != 0; $message = $exit_code == 0 ? "TRIM.SP-NL completed $run_base" - : "TRIM.SP-NL failed while running $run_base"; + : "TRIM.SP-NL failed while running $run_base ($completion)"; } # Return at most the latest 64 KiB to the browser and immediately remove the @@ -172,7 +511,8 @@ if ($request_logfile && -f $request_logfile && open(my $log_handle, "<", $reques unlink($request_logfile); } -print "Content-Type: application/json\r\n\r\n"; +print "Content-Type: application/json; charset=UTF-8\r\n"; +print "Cache-Control: no-store\r\n\r\n"; print encode_json({ ok => $ok, phase => $phase, @@ -182,4 +522,4 @@ print encode_json({ log => $log_content, }); -exit 0; +exit 0; \ No newline at end of file diff --git a/tests/cgi-tests.sh b/tests/cgi-tests.sh new file mode 100644 index 0000000..f654d59 --- /dev/null +++ b/tests/cgi-tests.sh @@ -0,0 +1,324 @@ +#!/usr/bin/env bash +# +# CGI hardening tests for singleTrimSP.cgi. +# +# Drives the real script directly (no web server) using REQUEST_METHOD / +# CONTENT_LENGTH / QUERY_STRING env vars, exactly like a CGI runtime would. +# A fake trimspNL engine provides success/fail/hang behavior so the watchdog, +# logging, and archive paths can be exercised deterministically. +# +# Usage: tests/cgi-tests.sh +# Exits non-zero on the first failing test. + +set -u + +HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +CGI="$HERE/../singleTrimSP.cgi" +ROOT_REPO="$(cd "$HERE/.." && pwd)" + +TMP="$(mktemp -d)" +trap 'rm -rf "$TMP"' EXIT + +ROOT="$TMP/root" # TRIMSP_TMPROOT for every request +mkdir -p "$ROOT" + +BIN="$TMP/bin/trimspNL" # fake engine +mkdir -p "$TMP/bin" + +cat > "$BIN" <<'EOF' +#!/bin/sh +case "${FAKE_MODE:-ok}" in + fail) + echo "fake engine failed as requested" 1>&2 + exit 3 + ;; + hang) + echo "$$" > "$FAKE_PIDFILE" + sleep 120 + exit 0 + ;; + ok) + base="$1" + printf '# col1 col2\n1 5\n2 10\n' > "$base.rge" + printf 'SEQ %s line1\nSEQ %s line2\n' "$base" "$base" > "$base.seq" + echo "fake engine completed $base" + exit 0 + ;; +esac +EOF +chmod +x "$BIN" + +PASS=0 +FAIL=0 + +note() { printf '\n== %s\n' "$*"; } +ok() { PASS=$((PASS+1)); printf ' PASS: %s\n' "$*"; } +bad() { FAIL=$((FAIL+1)); printf ' FAIL: %s\n' "$*"; } + +# run_request METHOD QUERY_STRING BODY_FILE -> sets RUN_HEADERS, RUN_BODY, RUN_STATUS +run_request() { + local method="$1" query="$2" body_file="$3" + local in_redir="/dev/null" + local BODY_LEN=0 + if [ "$method" = "POST" ] && [ -n "$body_file" ]; then + BODY_LEN="$(wc -c < "$body_file")" + in_redir="$body_file" + fi + local out + out="$(REQUEST_METHOD="$method" \ + CONTENT_TYPE="application/x-www-form-urlencoded" \ + CONTENT_LENGTH="$BODY_LEN" \ + QUERY_STRING="$query" \ + TRIMSP_TMPROOT="$ROOT" \ + TRIMSP_BIN="$BIN" \ + TRIMSP_CMD_TIMEOUT="4" \ + TRIMSP_JOB_MAXAGE="86400" \ + /usr/bin/perl -X "$CGI" < "$in_redir" 2>&1)" + # Split header block from body. + RUN_HEADERS="$(printf '%s\n' "$out" | sed -n '1,/^\r*$/p' | head -n -1)" + RUN_BODY="$(printf '%s\n' "$out" | sed -n '/^\r*$/,$p' | tail -n +1)" + # Without a status line the response is implicitly 200. + if printf '%s' "$RUN_HEADERS" | grep -q '^Status:'; then + RUN_STATUS="$(printf '%s' "$RUN_HEADERS" | sed -n 's/^Status: *\([0-9][0-9][0-9]\).*/\1/p' | head -1)" + else + RUN_STATUS="200" + fi +} + +body_contains() { printf '%s' "$RUN_BODY" | grep -q "$1"; } + +# Percent-encode a value the way the browser's URLSearchParams does. +urlencode() { + printf '%s' "$1" | perl -pe 's/([^A-Za-z0-9._~-])/sprintf("%%%02X", ord($1))/ge' +} + +# The CGI derives its path allowlist from TRIMSP_TMPROOT, so test bodies must +# reference the configured root (default /tmp in production). +FN_ROOT="$ROOT" + +# --------------------------------------------------------------------------- +note "health" +run_request GET "action=health" "" +if body_contains '"ok":true' && body_contains '"commandTimeoutSeconds"'; then + ok "health reports ok + settings" +else + bad "health response: $RUN_BODY" +fi + +# --------------------------------------------------------------------------- +note "path traversal is rejected" +TRAV="$TMP/trav.txt" +printf 'action=run&fn=%s&line0=x\n' "$(urlencode "$FN_ROOT/../../escape/x.inp")" > "$TRAV" +run_request POST "" "$TRAV" +if [ "$RUN_STATUS" = "400" ] && body_contains 'Invalid simulation filename'; then + ok "fn contains /../ component -> 400" +else + bad "traversal not rejected (status=$RUN_STATUS body=$RUN_BODY)" +fi + +printf 'action=run&fn=%s&line0=x\n' "$(urlencode "$FN_ROOT/../x.inp")" > "$TRAV" +run_request POST "" "$TRAV" +if [ "$RUN_STATUS" = "400" ]; then + ok "single-level /../ in run component -> 400" +else + bad "single-level traversal not rejected (status=$RUN_STATUS body=$RUN_BODY)" +fi + +printf 'action=run&fn=%s&line0=x\n' "$(urlencode "$FN_ROOT/run12345/x.inp")%0A" > "$TRAV" +run_request POST "" "$TRAV" +if [ "$RUN_STATUS" = "400" ]; then + ok "trailing newline (%0A) in fn rejected" +else + bad "%0A tail not rejected (status=$RUN_STATUS body=$RUN_BODY)" +fi + +# --------------------------------------------------------------------------- +note "write/read round-trip with special characters" +RUN=round1234 +CFG="$TMP/cfgbody" +printf 'fn=%s&line0=%s&line1=%s\n' \ + "$(urlencode "$FN_ROOT/$RUN/TrimSP.cfg")" \ + "ProjectileParameters" \ + "$(urlencode 'val=100¬e=semi+colon%,#hash')" \ + > "$CFG" +run_request POST "" "$CFG" +if [ "$RUN_STATUS" = "200" ] && body_contains '"phase":"save"'; then + ok "TrimSP.cfg written (phase=save)" +else + bad "cfg write failed (status=$RUN_STATUS body=$RUN_BODY)" +fi + +run_request GET "action=read&fn=$(urlencode "$FN_ROOT/$RUN/TrimSP.cfg")" "" +if body_contains 'val=100¬e=semi+colon%,#hash'; then + ok "read-back matches decoded content exactly (escaped +,%,&,#,=)" +else + bad "read-back mismatch: $RUN_BODY" +fi + +# Invalid read path (symlink-leading component) +ln -s /etc "$ROOT/$RUN/linkdir" 2>/dev/null || true +run_request GET "action=read&fn=$(urlencode "$FN_ROOT/$RUN/linkdir")" "" +if [ "$RUN_STATUS" = "404" ]; then + ok "read of non-regular file -> 404" +else + bad "read of symlink not rejected (status=$RUN_STATUS)" +fi + +# --------------------------------------------------------------------------- +note "engine success" +run_request POST "" "$TMP/cfgbody" # ensure run dir exists +INP="$TMP/inpbody" +printf 'fn=%s&rngType=pcg32&line0=X\n' "$(urlencode "$FN_ROOT/$RUN/${RUN}_E100.inp")" > "$INP" +FAKE_MODE=ok run_request POST "" "$INP" +if body_contains '"exitCode":0' && body_contains '"ok":true'; then + ok "engine completed OK" +else + bad "engine success not reported (body=$RUN_BODY)" +fi + +# The .seq file the fake engine wrote must be readable. +run_request GET "action=read&fn=$(urlencode "$FN_ROOT/$RUN/${RUN}_E100.seq")" "" +if body_contains 'SEQ'; then + ok "engine output .seq readable" +else + bad "engine .seq not readable (status=$RUN_STATUS body=$RUN_BODY)" +fi + +# --------------------------------------------------------------------------- +note "engine failure is reported, not silently ignored" +FAILINP="$TMP/failbody" +printf 'fn=%s&rngType=pcg32&line0=X\n' "$(urlencode "$FN_ROOT/$RUN/${RUN}_E200.inp")" > "$FAILINP" +FAKE_MODE=fail run_request POST "" "$FAILINP" +if body_contains '"exitCode":3' && body_contains '"ok":false'; then + ok "engine failure -> exitCode 3, ok=false" +else + bad "engine failure misreported (body=$RUN_BODY)" +fi + +# --------------------------------------------------------------------------- +note "watchdog kills a hung engine (alarm + process-group kill)" +HANGINP="$TMP/hangbody" +printf 'fn=%s&rngType=pcg32&line0=X\n' "$(urlencode "$FN_ROOT/$RUN/${RUN}_E300.inp")" > "$HANGINP" +HANG_PIDF="$TMP/pid.txt" +FAKE_MODE=hang FAKE_PIDFILE="$HANG_PIDF" TRIMSP_CMD_TIMEOUT="2" run_request POST "" "$HANGINP" +if ! [ -e "$HANG_PIDF" ]; then + bad "hang engine never recorded a pid (test setup broken)" +else + DEAD="$(kill -0 "$(cat "$HANG_PIDF")" 2>/dev/null && echo no || echo yes)" + if body_contains '"exitCode":124' && [ "$DEAD" = "yes" ]; then + ok "hung engine timed out -> exitCode 124, process group reaped" + else + bad "watchdog did not fire correctly (exit=$RUN_BODY dead=$DEAD)" + fi +fi + +# --------------------------------------------------------------------------- +note "archive + download" +# The sequence file triggers the archive of the whole run directory. +SEQ="$TMP/seqbody" +printf 'fn=%s&line0=S\n' "$(urlencode "$FN_ROOT/$RUN/${RUN}_Seq_Results.dat")" > "$SEQ" +run_request POST "" "$SEQ" +if body_contains '"archiveUrl"' && body_contains '"phase":"archive"' && [ ! -d "$ROOT/$RUN" ]; then + ok "archive created and run directory removed" +else + bad "archive flow broken (body=$RUN_BODY dir_gone=$( [ ! -d "$ROOT/$RUN" ] && echo yes || echo no))" +fi + +# Binary download: run through a file so NUL bytes survive. +REQ="$TMP/dlreq" +{ + printf 'action=download&run=%s' "$RUN" + printf '\n' +} > "$REQ" +DL="$TMP/dl.out" +REQUEST_METHOD=GET CONTENT_TYPE="application/x-www-form-urlencoded" \ + CONTENT_LENGTH=0 QUERY_STRING="action=download&run=$RUN" \ + TRIMSP_TMPROOT="$ROOT" TRIMSP_BIN="$BIN" \ + TRIMSP_CMD_TIMEOUT="4" TRIMSP_JOB_MAXAGE="86400" \ + /usr/bin/perl -X "$CGI" < /dev/null > "$DL" 2>&1 +if grep -a '^Status: 404' "$DL" >/dev/null; then + bad "download 404 for archived run" +elif grep -a '^Status:' "$DL" >/dev/null; then + bad "download unexpected error ($(grep -a '^Status:' "$DL" | head -1))" +else + DL_BODY="$TMP/dl.body" + perl -0777 -e ' + local $/; my $raw = <>; + my ($body) = $raw =~ /^.*?\r?\n\r?\n(.*)\z/s; + print STDOUT $body' "$DL" > "$DL_BODY" + if [ "$(dd if="$DL_BODY" bs=1 count=2 2>/dev/null | od -An -tx1 | tr -d ' \n')" = "1f8b" ]; then + ok "download returns a gzip stream" + else + bad "download did not return gzip (hex=$(dd if="$DL_BODY" bs=1 count=8 2>/dev/null | od -An -tx1 | tr -d ' \n'))" + fi +fi + +# --------------------------------------------------------------------------- +note "pruner removes stale run dirs and keeps fresh ones" +STALE=stale0001 +FRESH=fresh0001 +mkdir -p "$ROOT/$STALE" "$ROOT/$FRESH" +printf 'cfg\n' > "$ROOT/$STALE/TrimSP.cfg" +printf 'x\n' > "$ROOT/$STALE/extra" +printf 'cfg\n' > "$ROOT/$FRESH/TrimSP.cfg" +# Backdate AFTER populating so the mtime that matters (dir mtime) is stale. +touch -d "2000-01-01 00:00:00" "$ROOT/$STALE" +run_request GET "action=health" "" +if [ ! -d "$ROOT/$STALE" ] && [ -d "$ROOT/$FRESH" ]; then + ok "pruner removed stale run dir, kept fresh one" +else + bad "pruner misbehaved (stale_gone=$( [ ! -d "$ROOT/$STALE" ] && echo yes || echo no) fresh_kept=$( [ -d "$ROOT/$FRESH" ] && echo yes || echo no))" +fi + +# Orphaned archive pruned. +touch -d "2000-01-01 00:00:00" "$ROOT/orphan1234.tgz" +run_request GET "action=health" "" +if [ ! -e "$ROOT/orphan1234.tgz" ]; then + ok "pruner removed orphaned archive" +else + bad "orphaned archive not pruned" +fi + +# Non-trimsp directory in the shared root is never touched. +OTHER="$ROOT/other-user-000" +mkdir -p "$OTHER" +touch -d "2000-01-01 00:00:00" "$OTHER" +run_request GET "action=health" "" +if [ -d "$OTHER" ]; then + ok "unrelated directory left untouched" +else + bad "pruner removed unrelated directory" +fi + +# --------------------------------------------------------------------------- +note "symlinked run directory is refused" +mkdir -p "$TMP/real_dir" +ln -s "$TMP/real_dir" "$ROOT/symlnk001" +SYM="$TMP/symbody" +printf 'fn=%s&line0=x\n' "$(urlencode "$FN_ROOT/symlnk001/TrimSP.cfg")" > "$SYM" +run_request POST "" "$SYM" +if [ "$RUN_STATUS" = "500" ] && body_contains 'plain directory'; then + ok "symlinked run directory refused" +else + bad "symlinked run dir accepted (status=$RUN_STATUS body=$RUN_BODY)" +fi + +# --------------------------------------------------------------------------- +note "request size cap" +# The real client joins submitted lines on one body line with '&' separators. +BIG="$TMP/bigbody" +{ + printf 'fn=%s&rngType=pcg32&line0=z' "$(urlencode "$FN_ROOT/bigrun001/x.inp")" + for i in $(seq 1 20001); do printf '&line%d=z' "$i"; done + printf '\n' +} > "$BIG" +run_request POST "" "$BIG" +if [ "$RUN_STATUS" = "413" ]; then + ok "oversized request -> 413" +else + bad "oversized request not capped (status=$RUN_STATUS body=$RUN_BODY)" +fi + +# --------------------------------------------------------------------------- +printf '\n%d passed, %d failed\n' "$PASS" "$FAIL" +[ "$FAIL" -eq 0 ] \ No newline at end of file From 2e01232afdf0109aa6d94510bca03ada089a6850 Mon Sep 17 00:00:00 2001 From: salman Date: Sat, 26 Sep 2026 16:38:35 +0200 Subject: [PATCH 3/9] Exclude the archive lock marker from the downloaded results archive --- singleTrimSP.cgi | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/singleTrimSP.cgi b/singleTrimSP.cgi index 35bb3ca..c223bad 100644 --- a/singleTrimSP.cgi +++ b/singleTrimSP.cgi @@ -459,7 +459,8 @@ if ($filename =~ /_Seq_Results\.dat$/) { if ($ok) { my $command = "tar -czf " . shell_quote($archive_path) . " -C " . shell_quote($work_path) - . " . >> " . shell_quote($logfile) . " 2>&1"; + . " --exclude=./.trimsp-lock . " + . ">> " . shell_quote($logfile) . " 2>&1"; append_log($logfile, "\n[archive] \$ $command\n"); my $result = run_bounded_command($command); ($exit_code, my $completion) = describe_result($result); From 32a108734a0b60584796c6eed726bf0bf1cfca93 Mon Sep 17 00:00:00 2001 From: salman Date: Sat, 26 Sep 2026 17:06:09 +0200 Subject: [PATCH 4/9] Add validation limits --- ToDo.txt | 36 ++++++++++++++++++---------- TrimSPlib.js | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 90 insertions(+), 12 deletions(-) diff --git a/ToDo.txt b/ToDo.txt index 6e0c6c2..22f9c42 100644 --- a/ToDo.txt +++ b/ToDo.txt @@ -1,19 +1,31 @@ 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 +DONE - Validate ranSeed in the GUI and startSequence: it must be an integer in a + safe range (|v| <= 2^31-1, matching ISEED = IDINT(RI) in trimspNL.F). Enforced + in runSimulationSequence() (before any job is generated) AND in CreateInpFile() + (defense-in-depth), so a crafted share link or direct deck builder is blocked + with a "please change it" message. Non-numeric / NaN / non-integer / oversized + seeds no longer reach the engine. +DONE - 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. + (currently: initialSeed + iScan, only when the user set a valid seed). +DONE - Cap valEnergy magnitude at 60000 eV (positive incidence mode and the + Maxwellian transmission mode |E0| = temperature). Stopping fits (KDEE1=4) are + documented valid to ~50 keV for helium (meaning-of-params.txt:177-185), so 60 + keV is the model's fitted ceiling. Applied to scanned energies and the base + value in runSimulationSequence() and CreateInpFile(). +- 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 numberProj magnitude: an unbounded projectile count (e.g. + 1e9) does not terminate within the 300 s watchdog and can flood scan points + (empirically verified: 1e9 did not finish in 15 s). GUI ProjNumberLimit() is + currently dead code (defined at TrimSPlib.js:3516, never wired to an event). +- Persisting the effective per-point seed: the seed already lives in each + archived *.inp (all three deck slots), so reproducibility does not require the + *.seq header change; extending *.seq would shift the 999(A20,1x) layer slots + and break getSequenceColumnInfo(). Surfacing it in run details instead is + deferred - not sure it is needed. Deferred follow-ups from the C-list CGI hardening ================================================= diff --git a/TrimSPlib.js b/TrimSPlib.js index 0e8e4dd..c698610 100644 --- a/TrimSPlib.js +++ b/TrimSPlib.js @@ -3634,6 +3634,16 @@ function setValues(content) { function CreateInpFile() { + // Gate client-controlled magnitudes here too so a crafted config can never + // reach the engine, regardless of which caller builds the deck. + var inpSeedError = validateSeedString(All["ranSeed"]); + if (inpSeedError) { + throw new Error(inpSeedError); + } + var inpEnergyError = validateEnergyValue(All["valEnergy"]); + if (inpEnergyError) { + throw new Error(inpEnergyError); + } // Create and return an input deck for the TRIM.SP-NL transport engine. // Takes object All as input for all relevant parameters @@ -3781,6 +3791,45 @@ var TRIMSP_WEB_MAX_CONCURRENCY = 4; // an effectively unlimited list of jobs and freeze the tab. var TRIMSP_MAX_SCAN_POINTS = 5000; +// Validated limits for client-owned input values. The Ziegler/ICRU stopping +// fits are only tuned up to ~50 keV for helium (KDEE1=4, see +// meaning-of-params.txt:177-185), so energies beyond 60 keV are outside the +// model's fitted validity. Negative energies select the Maxwellian initial +// distribution where |E0| is used as the ion temperature (trimspNL.F:806), so +// the same magnitude cap applies to both modes. +var TRIMSP_ENERGY_CAP_EV = 60000; + +// The engine casts the deck seed to a signed 32-bit integer +// (ISEED = IDINT(RI), trimspNL.F:818-827), so any magnitude beyond that is +// silently truncated. Restrict the seed to |v| <= 2^31-1. +var TRIMSP_SEED_MAX_ABS = 2147483647; + +// Returns an empty string when valid, otherwise a message that asks the user +// to change the offending value. +function validateEnergyValue(raw) { + var str = String(raw).trim(); + var value = Number(str); + if (str === "" || !Number.isFinite(value)) { + return "Energy must be numeric. Please change it."; + } + if (Math.abs(value) > TRIMSP_ENERGY_CAP_EV) { + return "Energy magnitude must not exceed 60000 eV (the stopping fits are not validated beyond ~50 keV). Please change it."; + } + return ""; +} + +function validateSeedString(raw) { + var str = String(raw).trim(); + if (str === "" || !/^[+-]?\d+$/.test(str)) { + return "Seed must be an integer. Please change it."; + } + var value = Number(str); + if (Math.abs(value) > TRIMSP_SEED_MAX_ABS) { + return "Seed is out of range (use an integer between -2147483648 and 2147483647). Please change it."; + } + return ""; +} + // 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 @@ -3995,12 +4044,29 @@ async function runSimulationSequence() { return(0); } + // Cap the projectile energy magnitude before any job is generated. For an + // energy sweep the scan values are authoritative; for any other sweep the + // base valEnergy must still stay in the fitting range. + var evalues = (ScanAttrib === "valEnergy") ? SValues : [Number(All["valEnergy"])]; + for (var evIndex = 0; evIndex < evalues.length; evIndex++) { + var energyError = validateEnergyValue(evalues[evIndex]); + if (energyError) { + updateStatusBar({ run: energyError }); + return(0); + } + } + // Freeze the initial GUI state and expand the scan into independent jobs. // Each job gets its own input file name and a deterministic seed offset so // the results are reproducible even when runs finish out of order. const scanValues = SValues.slice(); const initialAll = Object.assign({}, All); const initialSeed = parseInt(initialAll["ranSeed"], 10); + const seedError = validateSeedString(initialAll["ranSeed"]); + if (seedError) { + updateStatusBar({ run: seedError }); + return(0); + } let jobs = []; for (let iScan = 0; iScan < scanValues.length; iScan++) { From 93f0b1c4d5d959f07e81f76779c6683ead6b7c96 Mon Sep 17 00:00:00 2001 From: Zaher Salman Date: Sat, 26 Sep 2026 18:55:38 +0200 Subject: [PATCH 5/9] Fix parIRL=1 crash: zero recoil cutoff makes the cascade diverge parIRL=1 crashed the engine with SIGSEGV (CGI exitCode 139) at both GUI defaults, so the sputtering path has never actually worked. The first diagnosis blamed KO(600,MAXNL5,2) being shallower than the 2000-wide recoil lists. That is wrong: enlarging KO to 2000 does not help, because the cascade overruns all of them. The real cause is parERC=0, which is the GUI default and what tests/Si_1layer/Si_1layer_E19000.inp carries. The primary knock-on test is IF(DEN(IV).LE.ERC) GO TO 6, so with no cutoff every atom receiving any energy is respawned as a recoil on each cascade generation; the branching ratio exceeds one and NREC1/NREC2 grow geometrically (81, 153, 269, 455, 773, 1317, ...) instead of draining, so the list-2/list-1 loop at label 83 never terminates. Forcing a real cutoff converges, with a peak depth of only 103, so KO was left at 600. fortran/trimspNL.F: - Refuse parIRL>=1 with parERC<=0 after readGuiInput, before any transport work, with a message naming the field to change. - The IF(NREC2.GT.2000) guard now stops non-zero instead of falling through into the out-of-bounds write loop, so any future non-convergent cascade fails cleanly instead of corrupting memory. TrimSPlib.js / TrimSP.html, so this is caught before the engine is invoked: - handleRecoilModeChange, wired to the parIRL widget, fills parERC with TRIMSP_MIN_RECOIL_CUTOFF = 0.02 when recoils are switched on with a cutoff of 0, and says so. - A pre-run check refuses the run if the user then insists on parIRL=1 with parERC=0, reporting via the status bar and a transient warning and focusing parERC. Deliberately not wired to parERC, so retyping the cutoff is what insisting means and is not silently overridden; this also covers shared config links, which assign widget values directly. - The parIRL tooltip documents the requirement. Tests: tests/recoil-cutoff-test.js (12 cases) lifts the shipped cutoff code out of TrimSPlib.js and runs it against a document stub. Verified against a fresh build: parERC=0/parIRL=1 now exits 1 with the new message instead of 139, parERC=0.02 completes with parIRL=0 or 1, and parERC=0/parIRL=0 output is unchanged (.rge byte-identical). tests/cgi-tests.sh 18/18. --- ToDo.txt | 87 +++++++++++++++++++++++++++-- TrimSP.html | 6 +- TrimSPlib.js | 87 +++++++++++++++++++++++++++++ fortran/trimspNL.F | 31 +++++++++++ tests/recoil-cutoff-test.js | 108 ++++++++++++++++++++++++++++++++++++ 5 files changed, 313 insertions(+), 6 deletions(-) create mode 100644 tests/recoil-cutoff-test.js diff --git a/ToDo.txt b/ToDo.txt index 22f9c42..2ba3f8f 100644 --- a/ToDo.txt +++ b/ToDo.txt @@ -17,16 +17,95 @@ DONE - Cap valEnergy magnitude at 60000 eV (positive incidence mode and the value in runSimulationSequence() and CreateInpFile(). - 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 numberProj magnitude: an unbounded projectile count (e.g. - 1e9) does not terminate within the 300 s watchdog and can flood scan points - (empirically verified: 1e9 did not finish in 15 s). GUI ProjNumberLimit() is - currently dead code (defined at TrimSPlib.js:3516, never wired to an event). +DONE (advisory, not a hard cap) - Warn on the web when numberProj exceeds 25000 + per point. Measured cost model on a 3.2 GHz core, single-threaded engine: + runtime is linear in numberProj and linear in energy, and cost per projectile + is dominated by the number of nuclear collisions along the track, so the + expensive axis is the LIGHT projectiles, not the heavy ones. Projectile -> + Si, Maxwellian 60 keV, ZBL potential, ms/projectile: muon 3.04, H 3.30, + He 4.06 (worst), Li-8 3.91, Be-11 3.64, B-12 3.19, Mg-31 1.34, Ar 0.83. + Checked and found NOT to affect runtime: layer count (1..60 layers is flat, + the species lookup is a binary search), depth increment dz (0.1 A == 20 A), + and grazing incidence (ALPHA 89 deg is cheaper - the ion skims out). + NH=25000 at the worst case measured 98.6/99.5/100.1 s on the 1.3.2 binary and + 101.5/100.7/101.7 s on a fresh 1.4.0 build, i.e. ~3x margin under the 300 s + TRIMSP_CMD_TIMEOUT. Deliberately warning-only: the value is advisory, the run + still starts, and the user keeps the final say. Web-only, because the + standalone app has no watchdog. TRIMSP_WEB_PROJ_ADVISORY in TrimSPlib.js; + uses the max of the sweep when scanning numberProj. The dead GUI + ProjNumberLimit() (TrimSPlib.js:3516) was left alone - it clamps at 5000, + which is below the 50000-100000 used by the publication test configs, so + wiring it up as-is would have clamped the regression suite. - Persisting the effective per-point seed: the seed already lives in each archived *.inp (all three deck slots), so reproducibility does not require the *.seq header change; extending *.seq would shift the 999(A20,1x) layer slots and break getSequenceColumnInfo(). Surfacing it in run details instead is deferred - not sure it is needed. +Out-of-bounds recoil write in the sputtering path (parIRL >= 1) +============================================================= + +FIXED - the engine now refuses the divergent input instead of corrupting memory. + + Symptom, as originally reported: setting parIRL (the recoil/sputter output + level) to 1 crashed the engine with SIGSEGV, reproducibly 5/5 seeds, and end + to end through singleTrimSP.cgi as exitCode 139 / ok:false. parIRL=0 was never + affected. + + Root cause was NOT the KO dimension. The first diagnosis blamed + KO(600,MAXNL5,2) at trimspNL.F:114 (debug: "Index '601' of dimension 1 of + array 'ko' above upper bound of 600"), on the grounds that the other + recoil-list arrays are 2000 wide. Enlarging KO to 2000 does not help: with + KO(2000) the run still dies, because the cascade overruns all of them. + + The actual cause is a non-converging cascade driven by parERC = 0. The + primary knock-on test is `IF(DEN(IV).LE.ERC) GO TO 6`, so with a zero recoil + cutoff every atom that receives any energy at all is respawned as a recoil on + every cascade generation. The branching ratio exceeds one, so NREC1 and NREC2 + grow geometrically instead of draining: + + NREC2 = 81, 153, 269, 455, 773, 1317, ... (parERC=0, parIRL=1) + + and the list-2/list-1 loop at label 83 never terminates. Instrumented runs + show the block is never even entered when parIRL=0. parERC defaults to 0.0 + in the GUI (TrimSP.html:293) and tests/Si_1layer/Si_1layer_E19000.inp carries + 0.0, so the failing combination is the default one - no unusual user action + is required to hit it. + + Evidence that a real cutoff converges: forcing parERC to 0.02, 0.05 or 0.2 + makes parIRL=1 finish normally (exit 0), and the peak cascade depth is only + 103, well inside KO's 600. KO was therefore left at 600, and the resize was + reverted (it cost 7.7 MiB of .lbss and fixed nothing). + + Changes in fortran/trimspNL.F: + - After readGuiInput, reject parIRL>=1 with parERC<=0 using a clear message + and STOP 1, before any transport work. + - The `IF(NREC2.GT.2000)` guard now stops with a non-zero status instead of + falling through into the out-of-bounds write loop, so any future + non-convergent cascade fails cleanly rather than corrupting memory. + + The GUI now catches this before the engine is ever invoked: + - handleRecoilModeChange (wired to the parIRL widget) fills parERC with + TRIMSP_MIN_RECOIL_CUTOFF = 0.02 when recoil calculation is switched on with + a cutoff of 0, and says so in a transient warning. + - A pre-run check in runSimulationSequence refuses the run if the user then + insists on parIRL=1 with parERC=0, reports it in the status bar plus a + transient warning, and focuses the parERC field. The handler is deliberately + not wired to parERC, so retyping the cutoff is what "insisting" means and is + not silently overridden. This also covers shared-configuration links, which + assign widget values directly and so bypass the change handler. + - The parIRL tooltip documents the requirement. + + Verified: parERC=0/parIRL=1 -> exit 1 with the new message; parERC=0.02 with + parIRL=0 or 1 -> exit 0; parERC=0 with parIRL=0 -> exit 0 and byte-identical + .rge (and an identical .out apart from timestamps/elapsed/filenames) versus + the pre-fix binary. tests/cgi-tests.sh 18/18, plus + tests/recoil-cutoff-test.js (12 cases over the shipped cutoff code) for the + GUI behaviour above. + + Minimal repro: tests/Si_1layer/Si_1layer_E19000.inp with the last field of + line 2 changed from 0 to 1 (parERC is field 10 of line 1, still 0.0). + Deferred follow-ups from the C-list CGI hardening ================================================= diff --git a/TrimSP.html b/TrimSP.html index 7fc8566..7ff11db 100644 --- a/TrimSP.html +++ b/TrimSP.html @@ -395,12 +395,14 @@ No recoils are generated (i.e., no sputtering effects); used to speed up the calculation if only projectile ranges are of interest. -
  • Calculate collision recoils.
  • +
  • Calculate collision recoils. Requires a + parERC recoil cutoff above zero; 0.02 is filled in + automatically.
  • - + diff --git a/TrimSPlib.js b/TrimSPlib.js index c698610..5fea487 100644 --- a/TrimSPlib.js +++ b/TrimSPlib.js @@ -3804,6 +3804,53 @@ var TRIMSP_ENERGY_CAP_EV = 60000; // silently truncated. Restrict the seed to |v| <= 2^31-1. var TRIMSP_SEED_MAX_ABS = 2147483647; +// Advisory projectile count for the web backend. This is not a hard limit: the +// engine is single threaded and its cost per projectile is dominated by the +// number of nuclear collisions along the track, so the same count can differ by +// more than 5x across the projectile menu. Measured worst case is helium into a +// light heavy-ish target at the 60 keV energy cap with the ZBL potential, at +// about 4.0 ms per projectile on a 3.2 GHz core, which puts 25000 points at +// roughly 100 s. The web backend watchdog (TRIMSP_CMD_TIMEOUT, 300 s) then +// still has about a 3x margin, so this count normally completes while slower or +// more heavily loaded deployments keep the watchdog as a backstop. The warning +// is advisory only; the user keeps the final say. +var TRIMSP_WEB_PROJ_ADVISORY = 25000; + +// Recoil cutoff substituted when the user turns on recoil calculation with a +// cutoff of zero. A zero cutoff makes the transport respawn every recoiling +// atom on every cascade generation, so NREC1/NREC2 grow geometrically instead +// of draining and the run overruns the recoil arrays. The engine refuses that +// combination, so the GUI supplies a usable value instead of letting the run +// fail after the user presses Run. +var TRIMSP_MIN_RECOIL_CUTOFF = 0.02; + +// True when the recoil calculation is enabled without a usable recoil cutoff. +// The parIRL and parERC widgets are plain number inputs and a shared +// configuration assigns their values directly, so this can be reached without +// going through the change handler below. +function recoilCutoffIsUnusable() { + let irl = Number(document.getElementById("parIRL").value); + let cutoff = Number(document.getElementById("parERC").value); + return Number.isFinite(irl) && irl >= 1 + && (!Number.isFinite(cutoff) || cutoff <= 0); +} + +// Called when the parIRL widget changes. Turning recoil calculation on with no +// cutoff is filled in for the user here, because that is a value they almost +// never intend to type. Deliberately not wired to parERC: retyping the cutoff +// afterwards is how a user insists on the combination, and the pre-run check +// then refuses the run instead of silently overriding them. +function handleRecoilModeChange() { + if (!recoilCutoffIsUnusable()) { + return; + } + let cutoffField = document.getElementById("parERC"); + cutoffField.value = TRIMSP_MIN_RECOIL_CUTOFF; + showTransientWarning( + "Recoil calculation needs a cutoff, so parERC was set to " + + TRIMSP_MIN_RECOIL_CUTOFF + ".", cutoffField, 6000); +} + // Returns an empty string when valid, otherwise a message that asks the user // to change the offending value. function validateEnergyValue(raw) { @@ -4056,6 +4103,46 @@ async function runSimulationSequence() { } } + // Advisory only, and web only: the standalone app runs the engine locally + // with no watchdog, so a long run is the user's own choice there. A number + // of projectiles that scales badly with the collision count can still exceed + // the backend watchdog, so warn once per run without blocking it. For a + // numberProj sweep the scan values are authoritative, otherwise the base + // value is what every point uses. + if (amIWeb()) { + const projValues = (ScanAttrib === "numberProj") + ? SValues + : [Number(All["numberProj"])]; + let worstProj = 0; + for (var projIndex = 0; projIndex < projValues.length; projIndex++) { + let projValue = Number(projValues[projIndex]); + if (Number.isFinite(projValue) && projValue > worstProj) { + worstProj = projValue; + } + } + if (worstProj > TRIMSP_WEB_PROJ_ADVISORY) { + showTransientWarning( + "Warning: " + worstProj + " projectiles per point may exceed the web " + + "time limit and fail. Runs above ~" + TRIMSP_WEB_PROJ_ADVISORY + + " are safest; the run will start anyway.", + document.getElementById("numberProj"), 8000); + } + } + + // Hard stop, unlike the projectile advisory above: with a zero recoil + // cutoff the cascade does not converge, so the engine exits with an error + // instead of producing results. Refuse the run here and say which field to + // change, rather than letting the backend fail with a transport message. + if (recoilCutoffIsUnusable()) { + let cutoffField = document.getElementById("parERC"); + let message = "parIRL=1 needs a parERC recoil cutoff above 0; " + + TRIMSP_MIN_RECOIL_CUTOFF + " is a common choice."; + updateStatusBar({ run: message }); + showTransientWarning("Not running: " + message, cutoffField, 8000); + cutoffField.focus(); + return(0); + } + // Freeze the initial GUI state and expand the scan into independent jobs. // Each job gets its own input file name and a deterministic seed offset so // the results are reproducible even when runs finish out of order. diff --git a/fortran/trimspNL.F b/fortran/trimspNL.F index 7e8369f..320315e 100644 --- a/fortran/trimspNL.F +++ b/fortran/trimspNL.F @@ -111,6 +111,12 @@ C & ,ISPIS(MAXNL5),ISPOP(MAXNL5),ISPOS(MAXNL5) INTEGER*4 ITSP(MAXNL5),ISPALT(MAXNL) & ,ISPIPT(MAXNL5),ISPIST(MAXNL5),ISPOPT(MAXNL5),ISPOST(MAXNL5) +C KO flags each recoil as a primary knock-on or a cascade product. Its +C first index is a recoil-list index, and 600 is deep enough for every +C cutoff that yields a converging cascade (measured peak is 103 with +C ERC=0.02). See the ERC/IRL check after readGuiInput below: with a zero +C recoil cutoff the cascade does not converge at all, and it overruns +C these 600 entries as well as the 2000-deep recoil lists. INTEGER*4 KO(600,MAXNL5,2) INTEGER*4 MEAB(MAXD2,22),MAGB(62,22),MEAGB(MAXD2,36,22) & ,MEABL(75,21),MEPB(MAXD2,MAXD2) @@ -569,6 +575,24 @@ C GUI file layout is isolated from the transport code. & ED,BE,CH1,CH2,CH3,CH4,CH5) +C A zero recoil cutoff makes the cascade diverge. The primary knock-on +C test below is IF(DEN(IV).LE.ERC) GO TO 6, so with ERC=0 every atom that +C receives any energy at all is respawned as a recoil on every cascade +C generation. The branching ratio then exceeds one, NREC1 and NREC2 grow +C geometrically instead of draining, and the run overruns the recoil +C arrays. Measured on tests/Si_1layer/Si_1layer_E19000.inp with +C parIRL=1: NREC2 passes 2000 within a single projectile, whereas a +C cutoff of 0.02 keeps the peak at 103. parERC defaults to 0.0 in the +C GUI, so this combination is reachable without the user doing anything +C unusual. Refuse it here, before any transport work, instead of +C corrupting memory later on. + if (irl.ge.1 .and. erc.le.0.d0) then + write(*,*) 'ERROR : parIRL=1 requires a positive parERC cutoff.' + write(*,*) ' parERC=0 makes the recoil cascade diverge' + write(*,*) ' and overrun the recoil arrays. Set parERC to' + write(*,*) ' a positive value (0.02 is a common choice).' + STOP 1 + endif C======================================================================= C OPEN OUTPUT FILES AND START CLOCK C======================================================================= @@ -1401,11 +1425,18 @@ C MAXA=MAX0(MAXA,NREC2) NALL=NALL+NREC2 NREC1=0 +C Guard against a cascade that does not fit the recoil lists. A +C non-converging cascade reaches this test with NREC2 far above any array +C bound, so the old fall-through into the processing loop below overran +C every recoil array and killed the run with SIGSEGV. A cascade that does +C not fit is not recoverable, so stop with a non-zero status rather than +C let the CGI report a truncated run as a success. IF(NREC2.GT.2000) GO TO 8885 GO TO 8886 8885 WRITE(6,8887) 8887 FORMAT(1X,'ERROR : DIMENSION IN THE RECOIL LOOP ', & 'MUST BE INCREASED') + STOP 1 8886 CONTINUE C C PROCESS THE PARTICLES IN LIST 2 diff --git a/tests/recoil-cutoff-test.js b/tests/recoil-cutoff-test.js new file mode 100644 index 0000000..ff65c15 --- /dev/null +++ b/tests/recoil-cutoff-test.js @@ -0,0 +1,108 @@ +// Checks the GUI handling of the parIRL/parERC combination that used to crash +// the engine. A zero recoil cutoff makes the transport cascade diverge, so the +// GUI supplies a cutoff when recoils are switched on and refuses the run if the +// user insists on the zero cutoff. +// +// The real functions are lifted out of TrimSPlib.js and run against a minimal +// document stub, so this exercises shipped code rather than a copy of it. +// Run with: node tests/recoil-cutoff-test.js + +const fs = require('fs'); +const path = require('path'); + +const libPath = path.join(__dirname, '..', 'TrimSPlib.js'); +const src = fs.readFileSync(libPath, 'utf8'); + +function grab(re) { + const m = src.match(re); + if (!m) { + throw new Error('pattern not found in ' + libPath + ': ' + re); + } + return m[0]; +} + +const fields = {}; +let shown = []; +let statusMsg = null; + +global.document = { + getElementById(id) { + if (!(id in fields)) { + fields[id] = { value: '', focus() { this.focused = true; } }; + } + return fields[id]; + } +}; +global.showTransientWarning = (message) => { shown.push(message); }; +global.updateStatusBar = (options) => { + if (options && options.run) { statusMsg = options.run; } +}; + +eval([ + grab(/var TRIMSP_MIN_RECOIL_CUTOFF = [\d.]+;/), + grab(/function recoilCutoffIsUnusable\(\) \{[\s\S]*?\n\}/), + grab(/function handleRecoilModeChange\(\) \{[\s\S]*?\n\}/), +].join('\n\n')); + +// The pre-run refusal, lifted verbatim out of runSimulationSequence. It keeps +// that function's return convention, where 0 means "aborted before the engine +// was invoked" and 1 means the run may proceed. +eval('function runGuardVerdict() {\n' + + grab(/ if \(recoilCutoffIsUnusable\(\)\) \{[\s\S]*?\n \}\n/) + + ' return 1;\n}\n'); + +function reset(irl, erc) { + for (const key of Object.keys(fields)) { delete fields[key]; } + const makeField = (value) => ({ + value: value, + focus() { this.focused = true; }, + }); + fields.parIRL = makeField(String(irl)); + fields.parERC = makeField(String(erc)); + shown = []; + statusMsg = null; +} + +const cases = [ + // description, IRL, ERC, touchIRL, fills, refuses + ['default GUI state', 0, '0.0', false, false, false], + ['parIRL 0->1, parERC=0 default', 1, '0.0', true, true, false], + ['user insists, edits parERC=0', 1, '0', false, false, true], + ['user sets parERC back to 0', 1, '0.0', false, false, true], + ['valid parIRL=1 with cutoff', 1, '0.02', false, false, false], + ['parIRL 0->1, cutoff already set', 1, '0.02', true, false, false], + ['valid parIRL=1, large cutoff', 1, '15', false, false, false], + ['parIRL=0 with parERC=0', 0, '0.0', false, false, false], + ['parIRL 0->1, negative parERC', 1, '-1', true, true, false], + ['parIRL 0->1, blank parERC', 1, '', true, true, false], + ['parIRL 0->1, non-numeric parERC', 1, 'abc', true, true, false], + ['non-numeric parIRL treated as off', 'x', '0.0', true, false, false], +]; + +let passed = 0; +let failed = 0; + +for (const [name, irl, erc, touchIRL, expectFilled, expectRefused] of cases) { + reset(irl, erc); + const before = fields.parERC.value; + // The handler is wired to the parIRL widget only, so it runs when the user + // changes parIRL and not when they go back and retype the cutoff themselves. + if (touchIRL) { handleRecoilModeChange(); } + const filled = fields.parERC.value !== before; + const refused = runGuardVerdict() === 0; + const refusedOk = (refused === expectRefused); + // A refused run must actually say something in both places. + const explained = !refused + || (shown.length > 0 && typeof statusMsg === 'string' && statusMsg.length > 0); + + const ok = (filled === expectFilled) && refusedOk && explained; + if (ok) { passed++; } else { failed++; } + console.log((ok ? 'PASS ' : 'FAIL ') + name.padEnd(34) + + 'parERC=' + String(fields.parERC.value).padEnd(6) + + (filled ? 'filled in' : 'left alone').padEnd(13) + + (refused ? 'refused' : 'runs') + + (shown.length ? ' | "' + shown[shown.length - 1] + '"' : '')); +} + +console.log('\n' + passed + ' passed, ' + failed + ' failed'); +process.exit(failed ? 1 : 0); From 04c7c6b1444754c0a029dfd5c185544c6f26127b Mon Sep 17 00:00:00 2001 From: Zaher Salman Date: Sat, 26 Sep 2026 19:06:51 +0200 Subject: [PATCH 6/9] Record follow-ups from the parIRL / parERC fix Adds a ToDo section for the three things deliberately left out of 93f0b1c, so they are not lost: - startSim() switches to the Results tab and purges the plots before runSimulationSequence() validates, so every refused run lands on an empty Results tab. Predates this work and affects all abort paths equally. - scripts/deploy-web.sh only rewrites TRIMSP_BIN in the CGI and does not build, so the engine-side fix needs a rebuild and install before it reaches the live site. The GUI refusal works immediately, so a deployment can briefly pair the friendly GUI message with an engine that still segfaults on a hand-written deck. - The untracked scratch in the tree (SrTiO3_E2000.inp, fortran/Makefile_org, fortran/profile*.txt, tests/{pcg32,ranlux,xoshiro}-test/) needs a decision on gitignore versus delete; .gitignore covers gmon.out and fortran/trimspNL-profile but not these. --- ToDo.txt | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/ToDo.txt b/ToDo.txt index 2ba3f8f..d749d54 100644 --- a/ToDo.txt +++ b/ToDo.txt @@ -123,4 +123,28 @@ Deferred follow-ups from the C-list CGI hardening key off exitCode === 124 and re-post the same .inp. - A future client change could surface TRIMSP_TMPROOT behind a shared configuration; the browser path regex is derived from the same env var, so - both sides stay consistent. \ No newline at end of file + both sides stay consistent. + +Follow-ups from the parIRL / parERC fix +======================================= + +- startSim() switches to the Results tab and purges the plots BEFORE + runSimulationSequence() validates anything, so every refused run drops the + user on an empty Results tab with no explanation of why. This predates the + parIRL work and affects all the abort paths equally (energy cap, seed + validation, recoil cutoff, and the re-entrancy guard in startSequence). Fix + by validating in startSim() before the tab switch, or by moving the tab + switch after the first successful job is queued. +- The web deployment runs TRIMSP_BIN, which defaults to /usr/local/bin/trimspNL, + so the engine-side fix in trimspNL.F does not reach the live site until the + binary is rebuilt and installed. scripts/deploy-web.sh only rewrites the path + in the CGI; it does not build. The GUI-side fix works immediately, so a + deployment can briefly have the friendly GUI refusal in front of an engine + that still segfaults on a hand-written deck. +- Untracked scratch left in the tree, decide .gitignore vs delete: + SrTiO3_E2000.inp (2 keV / 1000-projectile benchmark deck at the repo root), + fortran/Makefile_org (Makefile backup), fortran/profile.txt and + fortran/profile_annotated.txt (gprof reports; .gitignore covers gmon.out and + fortran/trimspNL-profile but not these), and tests/pcg32-test/, + tests/ranlux-test/, tests/xoshiro-test/ (per-backend engine run output trees + from the RNG comparison, i.e. generated results rather than fixtures). From a6f0b28cc5a86cac2cd13f48316ebbff88be3116 Mon Sep 17 00:00:00 2001 From: salman Date: Thu, 8 Oct 2026 16:18:11 +0200 Subject: [PATCH 7/9] Fix Electron plotting: expose Plotly as a global Electron wraps every renderer