diff options
| author | Hermes SysOp <sysop@webx11.duckdns.org> | 2026-11-12 16:24:15 +0100 |
|---|---|---|
| committer | Hermes SysOp <sysop@webx11.duckdns.org> | 2026-11-12 16:24:15 +0100 |
| commit | c08a795ae822e905639caf563e36cf7953d530f1 (patch) | |
| tree | bf4db2041002f4de721f95526c363ff186f30fe0 /modules/core | |
| parent | 8d0fe407b80bb822e8ef2852977afa3afbdc554a (diff) | |
fix(newos4): one arg-coercion boundary + binary-safe gunzip + clean boot log (v4.6.159)
1. Tool arguments are coerced once, at the dispatch boundary (module-aiassist.js)
- _coerceToolArgs(name, a) at :1584 reads the tool's own JSON schema and, for every
declared string prop, converts number/boolean with String() and object/array with
JSON.stringify (never "[object Object]"); declared integer/number props accept
unambiguous numeric strings. null/undefined stay absent so the handlers' defaults
and _pathArg still own "no value given".
- Called at both dispatch boundaries: runTool (:2650) and runModuleTool (:3565 —
reachable from view_module and from ship_module -> test_module/health_check, which
have their own parse+switch).
- A JSON body of "null" or "[1,2]" parses to a non-object; both sites normalise to {}
before any handler reads a.field.
- The 26 handlers keep their `a.x || ''` reads: `|| ''` catches only FALSY, which is
how a truthy number or object reached .trim()/.split()/.replace() and filed one
auto-report per tool (user_get "key.split is not a function", create_skill,
patch_skill, delegate_task "task.substring is not a function", create_mindmap,
web_fetch, list_skills, ship_module).
2. gunzip is binary-safe (cryptogzip.js — plan 2026-11-03 T3, the last open task)
- _cmdGunzip used _vfsRead + _gunzipString (base64-only), so a real .gz fetched with
wget reported "not valid base64 input (illegal character \u001f)" and could not be
decompressed at all. It now reads bytes (_vfsReadBytes handles both the module's
base64 output and raw Latin-1 bytes), checks the 1f 8b magic, and reports
"not a gzip file" for non-gzip input without writing or deleting anything.
The module's own gzip output still round-trips.
3. Boot log is clean (terminal.js)
- Seven _safeRegister() calls for Editor/Shell/Alias/Finder/FileManager/Diff/BusyBox
were guaranteed no-ops: those classes are declared in critical-ext-bundle.js, a
<script> after module-loader.js, so they do not exist yet when new WebX11Terminal()
runs; adding them to the registry map would still resolve to undefined. Removed with
a comment — the ext bundle registers each one on load.
- SimpleGUIModule IS a core class but was missing from _safeRegister's registry map,
so the fallback could never restore it. Added.
4. New guard: admin/scripts/verify-plan-age.py + profile wrapper plan-age-guard.sh
(cron newos4-plan-age-guard, no_agent, daily 07:10). Silent when healthy; reports
plans marked <!-- autonomy: review --> with no execution stamp older than 72 h —
the process gap that let four fix plans sit for six days before v4.6.158. Read-only
by construction (148 real plans byte+mtime identical before/after a run), rc=2 when
the directory cannot be scanned, --strict opt-in so a report is never recorded as a
crashed guard.
Verified: 37/37 node tests, 3 new test files and 631 new assertions (613 arg-coercion,
18 gunzip); test-cryptogzip-guard's C8 updated because it pinned the old misleading
"not valid base64" message. Independent orchestrator checks: all 66 declared string
props across 58 tool defs coerced under 4 hostile shapes; positive cases untouched
(string content not double-encoded, numbers still numbers, null still absent, unknown
tool identity preserved). Served bytes byte-identical for core-bundle.js, main.html and
cryptogzip.js. Live Chromium on v4.6.159: write_file with object content wrote 27 chars
of readable JSON (not "[object Object]"), read_file with offset:"0"/limit:"200" coerced
to numbers, user_set with key 12345 -> "12345" and object value -> JSON, and the boot
log now carries 0 SafeRegister warnings (52 messages, 0 JS errors) with SimpleGUIModule
registered.
Diffstat (limited to 'modules/core')
| -rw-r--r-- | modules/core/core-bundle.js | 78 | ||||
| -rw-r--r-- | modules/core/module-aiassist.js | 60 | ||||
| -rw-r--r-- | modules/core/module-loader.js | 2 | ||||
| -rw-r--r-- | modules/core/terminal.js | 18 |
4 files changed, 141 insertions, 17 deletions
diff --git a/modules/core/core-bundle.js b/modules/core/core-bundle.js index 69b5e1a..b365fa0 100644 --- a/modules/core/core-bundle.js +++ b/modules/core/core-bundle.js @@ -4080,14 +4080,15 @@ Only output XML function calls for tool tasks. If no tool is needed, answer norm // bump LATEST_VER + main.html ?v=, re-run admin/scripts/probe-ui-modes.py. // Tracked in .hermes/plans/2026-11-09_*-newos4-ui-rollback.md // this._safeRegister('UIShellModule', 'UI shell'); - // v4.5.47: Critical ext modules promoted to boot path - this._safeRegister('EditorModule', 'editor'); - this._safeRegister('ShellModule', 'shell'); - this._safeRegister('AliasModule', 'aliases'); - this._safeRegister('FinderModule', 'finder'); - this._safeRegister('FileManagerModule', 'file manager'); - this._safeRegister('DiffModule', 'diff'); - this._safeRegister('BusyBoxModule', 'busybox'); + // v4.5.47 used to _safeRegister() seven critical ext classes here (EditorModule, + // ShellModule, AliasModule, FinderModule, FileManagerModule, DiffModule, + // BusyBoxModule). They are declared in modules/ext/critical-ext-bundle.js, which is + // a <script> AFTER module-loader.js — i.e. it does not exist yet when + // `new WebX11Terminal()` runs, so every one of those calls was a guaranteed no-op + // that only emitted "[SafeRegister] Skipped … is not in the registry map" on each + // page load (adding them to the registry map would still resolve to undefined). + // critical-ext-bundle.js owns them and registers each one itself on load. + // Do NOT re-add them here — that just restores the warnings. } _initRegisteredModules() { for (const [name, mod] of Object.entries(this.moduleManager.modules)) { @@ -4127,6 +4128,7 @@ Only output XML function calls for tool tasks. If no tool is needed, answer norm DiffModule: typeof DiffModule !== 'undefined' ? DiffModule : undefined, BusyBoxModule: typeof BusyBoxModule !== 'undefined' ? BusyBoxModule : undefined, SmartContextModule: typeof SmartContextModule !== 'undefined' ? SmartContextModule : undefined, + SimpleGUIModule: typeof SimpleGUIModule !== 'undefined' ? SimpleGUIModule : undefined, DebugSupervisorModule: typeof DebugSupervisorModule !== 'undefined' ? DebugSupervisorModule : undefined, UIShellModule: typeof UIShellModule !== 'undefined' ? UIShellModule : undefined, BillingModule: typeof BillingModule !== 'undefined' ? BillingModule : undefined, @@ -10518,6 +10520,54 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re _pathArg(v) { return typeof v === 'string' ? v : (v == null ? '' : String(v)); } + // Single boundary coercion for tool arguments. The schemas already say + // which args are strings, but models send numbers/arrays/objects anyway + // and a handler's `a.field || ''` only catches FALSY values — so + // `.trim()`/`.split()`/`.replace()` on a truthy number or object threw a + // TypeError and the call was lost (one filed auto-report per tool). + // Coerce once here, per declared schema type, instead of patching ~26 + // handlers. Never throws: an unknown tool, bad defs or a non-object + // argument all fall back to the original value. + _coerceToolArgs(name, a) { + try { + if (!a || typeof a !== 'object' || Array.isArray(a)) return a; + var defs = this._toolDefs; + if (!defs) { defs = this._buildToolDefs(); this._toolDefs = defs; } + if (!Array.isArray(defs)) return a; + var props = null; + for (var i = 0; i < defs.length; i++) { + var fn = defs[i] && defs[i].function; + if (fn && fn.name === name) { + props = (fn.parameters && fn.parameters.properties) || null; + break; + } + } + if (!props) return a; + for (var p in props) { + if (!Object.prototype.hasOwnProperty.call(a, p)) continue; + var v = a[p]; + // Absent/null stays absent: _pathArg and the handlers' own + // defaults still own "no value given". + if (v === null || v === undefined) continue; + var type = props[p] && props[p].type; + if (type === 'string') { + if (typeof v === 'string') continue; + if (typeof v === 'number' || typeof v === 'boolean') { + a[p] = String(v); + } else if (typeof v === 'object') { + // Structured content must reach the file as readable + // JSON, never "[object Object]". + try { a[p] = JSON.stringify(v); } catch (_e) { a[p] = String(v); } + } + } else if (type === 'integer' || type === 'number') { + // Only convert unambiguous numeric strings; anything else + // is left for the handler to validate/report. + if (typeof v === 'string' && /^-?\d+(\.\d+)?$/.test(v)) a[p] = Number(v); + } + } + return a; + } catch (_e) { return a; } + } _updateDebugPanel() { if (!this._debugPanelEnabled) return; const panel = document.getElementById('debug-ai-panel'); @@ -11538,6 +11588,13 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re this.terminal.print(`🟡 GUARDRAIL WARNING: ${guardrail.description} (rule: ${guardrail.id})`, 'warning'); } a = typeof argsStr === 'object' ? argsStr : (argsStr ? JSON.parse(argsStr) : {}); + // A body of "null" / "[1,2]" parses to a non-object, and every + // handler below reads a.field — normalise that once here. + if (!a || typeof a !== 'object' || Array.isArray(a)) a = {}; + // ONE schema-driven coercion for the whole switch: a model that + // sends a number/array/object for a schema-string field must not + // reach `.trim()`/`.split()` still holding that type. + a = this._coerceToolArgs(name, a); switch (name) { case 'list_files': { const originalPath = this._pathArg(a.path); @@ -12448,6 +12505,11 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re let a = {}; try { a = typeof argsStr === 'object' ? argsStr : (argsStr ? JSON.parse(argsStr) : {}); + if (!a || typeof a !== 'object' || Array.isArray(a)) a = {}; + // Same single boundary as runTool: module tools take a string + // `name`/`code`/`file` too and are reachable directly and via + // runTool's `case 'view_module':` delegation. + a = this._coerceToolArgs(name, a); switch (name) { case 'view_module': { const modName = a.name || ''; diff --git a/modules/core/module-aiassist.js b/modules/core/module-aiassist.js index fb4329f..a9b8fcc 100644 --- a/modules/core/module-aiassist.js +++ b/modules/core/module-aiassist.js @@ -1573,6 +1573,54 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re _pathArg(v) { return typeof v === 'string' ? v : (v == null ? '' : String(v)); } + // Single boundary coercion for tool arguments. The schemas already say + // which args are strings, but models send numbers/arrays/objects anyway + // and a handler's `a.field || ''` only catches FALSY values — so + // `.trim()`/`.split()`/`.replace()` on a truthy number or object threw a + // TypeError and the call was lost (one filed auto-report per tool). + // Coerce once here, per declared schema type, instead of patching ~26 + // handlers. Never throws: an unknown tool, bad defs or a non-object + // argument all fall back to the original value. + _coerceToolArgs(name, a) { + try { + if (!a || typeof a !== 'object' || Array.isArray(a)) return a; + var defs = this._toolDefs; + if (!defs) { defs = this._buildToolDefs(); this._toolDefs = defs; } + if (!Array.isArray(defs)) return a; + var props = null; + for (var i = 0; i < defs.length; i++) { + var fn = defs[i] && defs[i].function; + if (fn && fn.name === name) { + props = (fn.parameters && fn.parameters.properties) || null; + break; + } + } + if (!props) return a; + for (var p in props) { + if (!Object.prototype.hasOwnProperty.call(a, p)) continue; + var v = a[p]; + // Absent/null stays absent: _pathArg and the handlers' own + // defaults still own "no value given". + if (v === null || v === undefined) continue; + var type = props[p] && props[p].type; + if (type === 'string') { + if (typeof v === 'string') continue; + if (typeof v === 'number' || typeof v === 'boolean') { + a[p] = String(v); + } else if (typeof v === 'object') { + // Structured content must reach the file as readable + // JSON, never "[object Object]". + try { a[p] = JSON.stringify(v); } catch (_e) { a[p] = String(v); } + } + } else if (type === 'integer' || type === 'number') { + // Only convert unambiguous numeric strings; anything else + // is left for the handler to validate/report. + if (typeof v === 'string' && /^-?\d+(\.\d+)?$/.test(v)) a[p] = Number(v); + } + } + return a; + } catch (_e) { return a; } + } _updateDebugPanel() { if (!this._debugPanelEnabled) return; const panel = document.getElementById('debug-ai-panel'); @@ -2593,6 +2641,13 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re this.terminal.print(`🟡 GUARDRAIL WARNING: ${guardrail.description} (rule: ${guardrail.id})`, 'warning'); } a = typeof argsStr === 'object' ? argsStr : (argsStr ? JSON.parse(argsStr) : {}); + // A body of "null" / "[1,2]" parses to a non-object, and every + // handler below reads a.field — normalise that once here. + if (!a || typeof a !== 'object' || Array.isArray(a)) a = {}; + // ONE schema-driven coercion for the whole switch: a model that + // sends a number/array/object for a schema-string field must not + // reach `.trim()`/`.split()` still holding that type. + a = this._coerceToolArgs(name, a); switch (name) { case 'list_files': { const originalPath = this._pathArg(a.path); @@ -3503,6 +3558,11 @@ max_chars: { type: 'integer', description: 'Max characters of page content to re let a = {}; try { a = typeof argsStr === 'object' ? argsStr : (argsStr ? JSON.parse(argsStr) : {}); + if (!a || typeof a !== 'object' || Array.isArray(a)) a = {}; + // Same single boundary as runTool: module tools take a string + // `name`/`code`/`file` too and are reachable directly and via + // runTool's `case 'view_module':` delegation. + a = this._coerceToolArgs(name, a); switch (name) { case 'view_module': { const modName = a.name || ''; diff --git a/modules/core/module-loader.js b/modules/core/module-loader.js index 43c7e06..b926c4a 100644 --- a/modules/core/module-loader.js +++ b/modules/core/module-loader.js @@ -4,7 +4,7 @@ // Core module bundle loaded synchronously in main.html. // This loader only handles version cache-busting, splash, terminal init, and optional ext modules. - const LATEST_VER = 'v4.6.158'; + const LATEST_VER = 'v4.6.159'; const CACHE = '?v=' + LATEST_VER; // Exposed so ext modules can register their own URL with the same cache-busting version // instead of an unversioned path the browser may already have cached (shell.js did). diff --git a/modules/core/terminal.js b/modules/core/terminal.js index 91bea4c..d0f6cff 100644 --- a/modules/core/terminal.js +++ b/modules/core/terminal.js @@ -1391,14 +1391,15 @@ // bump LATEST_VER + main.html ?v=, re-run admin/scripts/probe-ui-modes.py. // Tracked in .hermes/plans/2026-11-09_*-newos4-ui-rollback.md // this._safeRegister('UIShellModule', 'UI shell'); - // v4.5.47: Critical ext modules promoted to boot path - this._safeRegister('EditorModule', 'editor'); - this._safeRegister('ShellModule', 'shell'); - this._safeRegister('AliasModule', 'aliases'); - this._safeRegister('FinderModule', 'finder'); - this._safeRegister('FileManagerModule', 'file manager'); - this._safeRegister('DiffModule', 'diff'); - this._safeRegister('BusyBoxModule', 'busybox'); + // v4.5.47 used to _safeRegister() seven critical ext classes here (EditorModule, + // ShellModule, AliasModule, FinderModule, FileManagerModule, DiffModule, + // BusyBoxModule). They are declared in modules/ext/critical-ext-bundle.js, which is + // a <script> AFTER module-loader.js — i.e. it does not exist yet when + // `new WebX11Terminal()` runs, so every one of those calls was a guaranteed no-op + // that only emitted "[SafeRegister] Skipped … is not in the registry map" on each + // page load (adding them to the registry map would still resolve to undefined). + // critical-ext-bundle.js owns them and registers each one itself on load. + // Do NOT re-add them here — that just restores the warnings. } _initRegisteredModules() { for (const [name, mod] of Object.entries(this.moduleManager.modules)) { @@ -1438,6 +1439,7 @@ DiffModule: typeof DiffModule !== 'undefined' ? DiffModule : undefined, BusyBoxModule: typeof BusyBoxModule !== 'undefined' ? BusyBoxModule : undefined, SmartContextModule: typeof SmartContextModule !== 'undefined' ? SmartContextModule : undefined, + SimpleGUIModule: typeof SimpleGUIModule !== 'undefined' ? SimpleGUIModule : undefined, DebugSupervisorModule: typeof DebugSupervisorModule !== 'undefined' ? DebugSupervisorModule : undefined, UIShellModule: typeof UIShellModule !== 'undefined' ? UIShellModule : undefined, BillingModule: typeof BillingModule !== 'undefined' ? BillingModule : undefined, |