diff options
| author | Agando <roduccomm@gmail.com> | 2026-10-27 12:32:37 +0100 |
|---|---|---|
| committer | Agando <roduccomm@gmail.com> | 2026-10-27 12:32:37 +0100 |
| commit | 9a61780447066919964b66cdbaa35b9165629419 (patch) | |
| tree | a96a40371314705317e8cc8473c8d32690ff1b39 | |
| parent | 97279c0eb918701ac915187f5deb9f0adf9236bd (diff) | |
fix(newos4): never overwrite a directory node; ls honours a path after its flags
MAINT-010 and MAINT-016, the two high items the review left open. Both were re-probed
before the change and the new test is red on HEAD (5 passed, 7 FAILED).
- virtual-fs.js rename()/copy(): they assigned dst.dir.content[dst.name] with no destination
type check, so `mv file /docs` replaced the /docs node and every child vanished while the
command printed "Moved: …". Refuse when the destination is a real directory node (the
virtual /sys entry is exempt). rename() also erased the file on `mv a a`, because assigning
and then deleting the same key removes it; that path is now an explicit no-op.
- module-fs.js mv/cp: a refusal used to surface as "No such file or directory", which sent
the operator looking for a missing source. The destination is now inspected first and the
message says "is a directory — refusing to overwrite it".
- module-fs.js ls: the path was only read from argv[0], so `ls -l /work` listed the cwd.
The first non-flag argument is now the target; the path-first form is unchanged.
Tests: modules/test/test-fs-mv-cp-guard.js (12 asserts) — RED on HEAD (A1-A3, B1, B2, C2, E1),
GREEN now; maintain --fix reports "node tests 7/7 pass". Browser on v4.6.104:
"mv: /vtest: is a directory — refusing to overwrite it", child and source intact,
"ls -l /vtest" lists /vtest (keep.txt), not the cwd.
| -rw-r--r-- | main.html | 6 | ||||
| -rw-r--r-- | modules/core/core-bundle.js | 29 | ||||
| -rw-r--r-- | modules/core/module-fs.js | 21 | ||||
| -rw-r--r-- | modules/core/module-loader.js | 2 | ||||
| -rw-r--r-- | modules/core/virtual-fs.js | 8 | ||||
| -rw-r--r-- | modules/test/test-fs-mv-cp-guard.js | 109 |
6 files changed, 163 insertions, 12 deletions
@@ -1349,13 +1349,13 @@ </script> <!-- v4.5.63: A2A scratchpad fixes + agent-register arg fix --> -<script src="modules/core/core-bundle.js?v=v4.6.103"></script> +<script src="modules/core/core-bundle.js?v=v4.6.104"></script> <!-- Async dependency-ordered module loader now only initializes terminal and loads optional ext modules --> -<script src="modules/core/module-loader.js?v=v4.6.103"></script> +<script src="modules/core/module-loader.js?v=v4.6.104"></script> <!-- Critical ext bundle loads AFTER webx11 exists so registerModule calls work --> -<script src="modules/ext/critical-ext-bundle.js?v=v4.6.103"></script> +<script src="modules/ext/critical-ext-bundle.js?v=v4.6.104"></script> <script> // ═══ ONE TRUE VERSION SOURCE ═══ diff --git a/modules/core/core-bundle.js b/modules/core/core-bundle.js index 84d194d..d40988e 100644 --- a/modules/core/core-bundle.js +++ b/modules/core/core-bundle.js @@ -688,6 +688,11 @@ class VirtualFileSystem { if (!dst || !dst.name) return false; const node = src.dir.content[src.name]; if (!node) return false; + // mv a a is a no-op, not a delete (assign-then-delete on the same key used to erase it) + if (src.dir === dst.dir && src.name === dst.name) return true; + const existing = dst.dir.content[dst.name]; + // Never clobber a directory node: its children would vanish with it (MAINT-010). + if (existing && existing.type === 'dir' && !existing._virtual) return false; dst.dir.content[dst.name] = node; delete src.dir.content[src.name]; this.saveToStorage(); @@ -703,6 +708,9 @@ class VirtualFileSystem { if (!dst || !dst.name) return false; const node = src.dir.content[src.name]; if (!node) return false; + const existing = dst.dir.content[dst.name]; + // Never clobber a directory node: its children would vanish with it (MAINT-010). + if (existing && existing.type === 'dir' && !existing._virtual) return false; dst.dir.content[dst.name] = JSON.parse(JSON.stringify(node)); this.saveToStorage(); this._emitChange(dstPath); @@ -6482,8 +6490,11 @@ class FileSystemModule { registerCommands() { const self = this; this.commands['ls'] = (args) => { - const target = args && args.length > 0 && !args[0].startsWith('-') ? args[0] : undefined; - const flags = (args || []).filter(a => a.startsWith('-')); + const argv = args || []; + // v4.6.104: the path may follow the flags — `ls -l /work` used to list the cwd + // because only argv[0] was considered (MAINT-016). + const flags = argv.filter(a => a.startsWith('-')); + const target = argv.find(a => !a.startsWith('-')); const showLong = flags.some(f => f.includes('l')); const showHidden = flags.some(f => f.includes('a')); const files = this.terminal.fs.ls(target); @@ -6618,14 +6629,24 @@ class FileSystemModule { }; this.commands['mv'] = (args) => { if(!args[0] || !args[1]) return this.terminal.print('Usage: mv <source> <dest>', 'warning'); - if (this.terminal.fs.mv(args[0], args[1])) + const fs = this.terminal.fs; + const d = fs._resolvePath(args[1]); + const dn = d && d.name ? d.dir.content[d.name] : null; + if (dn && dn.type === 'dir' && !dn._virtual) + return this.terminal.print('mv: ' + args[1] + ': is a directory — refusing to overwrite it', 'error'); + if (fs.mv(args[0], args[1])) this.terminal.print('Moved: ' + args[0] + ' to ' + args[1], 'success'); else this.terminal.print('mv: ' + args[0] + ': No such file or directory', 'error'); }; this.commands['cp'] = (args) => { if(!args[0] || !args[1]) return this.terminal.print('Usage: cp <source> <dest>', 'warning'); - if (this.terminal.fs.cp(args[0], args[1])) + const fs = this.terminal.fs; + const d = fs._resolvePath(args[1]); + const dn = d && d.name ? d.dir.content[d.name] : null; + if (dn && dn.type === 'dir' && !dn._virtual) + return this.terminal.print('cp: ' + args[1] + ': is a directory — refusing to overwrite it', 'error'); + if (fs.cp(args[0], args[1])) this.terminal.print('Copied: ' + args[0] + ' to ' + args[1], 'success'); else this.terminal.print('cp: ' + args[0] + ': No such file or directory', 'error'); diff --git a/modules/core/module-fs.js b/modules/core/module-fs.js index f2b215a..583cfda 100644 --- a/modules/core/module-fs.js +++ b/modules/core/module-fs.js @@ -44,8 +44,11 @@ class FileSystemModule { registerCommands() { const self = this; this.commands['ls'] = (args) => { - const target = args && args.length > 0 && !args[0].startsWith('-') ? args[0] : undefined; - const flags = (args || []).filter(a => a.startsWith('-')); + const argv = args || []; + // v4.6.104: the path may follow the flags — `ls -l /work` used to list the cwd + // because only argv[0] was considered (MAINT-016). + const flags = argv.filter(a => a.startsWith('-')); + const target = argv.find(a => !a.startsWith('-')); const showLong = flags.some(f => f.includes('l')); const showHidden = flags.some(f => f.includes('a')); const files = this.terminal.fs.ls(target); @@ -180,14 +183,24 @@ class FileSystemModule { }; this.commands['mv'] = (args) => { if(!args[0] || !args[1]) return this.terminal.print('Usage: mv <source> <dest>', 'warning'); - if (this.terminal.fs.mv(args[0], args[1])) + const fs = this.terminal.fs; + const d = fs._resolvePath(args[1]); + const dn = d && d.name ? d.dir.content[d.name] : null; + if (dn && dn.type === 'dir' && !dn._virtual) + return this.terminal.print('mv: ' + args[1] + ': is a directory — refusing to overwrite it', 'error'); + if (fs.mv(args[0], args[1])) this.terminal.print('Moved: ' + args[0] + ' to ' + args[1], 'success'); else this.terminal.print('mv: ' + args[0] + ': No such file or directory', 'error'); }; this.commands['cp'] = (args) => { if(!args[0] || !args[1]) return this.terminal.print('Usage: cp <source> <dest>', 'warning'); - if (this.terminal.fs.cp(args[0], args[1])) + const fs = this.terminal.fs; + const d = fs._resolvePath(args[1]); + const dn = d && d.name ? d.dir.content[d.name] : null; + if (dn && dn.type === 'dir' && !dn._virtual) + return this.terminal.print('cp: ' + args[1] + ': is a directory — refusing to overwrite it', 'error'); + if (fs.cp(args[0], args[1])) this.terminal.print('Copied: ' + args[0] + ' to ' + args[1], 'success'); else this.terminal.print('cp: ' + args[0] + ': No such file or directory', 'error'); diff --git a/modules/core/module-loader.js b/modules/core/module-loader.js index 18137ec..0d61710 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.103'; + const LATEST_VER = 'v4.6.104'; const CACHE = '?v=' + LATEST_VER; const stored = localStorage.getItem('webx11_cache_ver'); if (stored !== LATEST_VER) { diff --git a/modules/core/virtual-fs.js b/modules/core/virtual-fs.js index 3ed1b40..3a3e119 100644 --- a/modules/core/virtual-fs.js +++ b/modules/core/virtual-fs.js @@ -686,6 +686,11 @@ class VirtualFileSystem { if (!dst || !dst.name) return false; const node = src.dir.content[src.name]; if (!node) return false; + // mv a a is a no-op, not a delete (assign-then-delete on the same key used to erase it) + if (src.dir === dst.dir && src.name === dst.name) return true; + const existing = dst.dir.content[dst.name]; + // Never clobber a directory node: its children would vanish with it (MAINT-010). + if (existing && existing.type === 'dir' && !existing._virtual) return false; dst.dir.content[dst.name] = node; delete src.dir.content[src.name]; this.saveToStorage(); @@ -701,6 +706,9 @@ class VirtualFileSystem { if (!dst || !dst.name) return false; const node = src.dir.content[src.name]; if (!node) return false; + const existing = dst.dir.content[dst.name]; + // Never clobber a directory node: its children would vanish with it (MAINT-010). + if (existing && existing.type === 'dir' && !existing._virtual) return false; dst.dir.content[dst.name] = JSON.parse(JSON.stringify(node)); this.saveToStorage(); this._emitChange(dstPath); diff --git a/modules/test/test-fs-mv-cp-guard.js b/modules/test/test-fs-mv-cp-guard.js new file mode 100644 index 0000000..369882d --- /dev/null +++ b/modules/test/test-fs-mv-cp-guard.js @@ -0,0 +1,109 @@ +// modules/test/test-fs-mv-cp-guard.js — run: node +// Proves MAINT-010 (mv/cp must not destroy a directory node) and MAINT-016 (ls -l <path>) +// against the real classes, no browser. +const fsMod = require('fs'), path = require('path'); +const ROOT = path.join(__dirname, '..', 'core'); +const ls = {}; +global.localStorage = { getItem: k => (k in ls ? ls[k] : null), setItem: (k, v) => { ls[k] = String(v); }, removeItem: k => { delete ls[k]; } }; +global.indexedDB = { open() { throw new Error('no idb'); }, deleteDatabase() { throw new Error('no idb'); } }; +global.window = global; + +function load(file, cls) { + return new Function(fsMod.readFileSync(path.join(ROOT, file), 'utf8') + '\nreturn ' + cls + ';')(); +} +const VFS = load('virtual-fs.js', 'VirtualFileSystem'); +const FSModule = load('module-fs.js', 'FileSystemModule'); + +function mk() { + const out = []; + const vfs = new VFS(); + vfs.saveToStorage = () => true; + const term = { print: (m, t) => out.push({ text: String(m), type: t || '' }), fs: vfs }; + const mod = new FSModule(); + mod.terminal = term; + mod.registerCommands(); + return { vfs, mod, out }; +} + +let fails = 0, passes = 0; +function ok(name, cond, detail) { + if (cond) { passes++; return; } + fails++; + console.log('FAIL ' + name + (detail ? ' — ' + detail : '')); +} + +// A. mv a file over a directory must refuse and leave the children alone +{ + const { vfs, mod, out } = mk(); + vfs.mkdirRecursive('/docs'); + vfs.writeFile('/docs/keep.txt', 'precious'); + vfs.writeFile('/report.txt', 'x'); + out.length = 0; + mod.commands['mv'](['/report.txt', '/docs']); + const msg = out.map(o => o.text).join(' '); + ok('A1 mv onto a directory is refused', /is a directory/.test(msg), msg); + ok('A2 destination children intact', vfs.readFile('/docs/keep.txt') === 'precious'); + ok('A3 source still there', vfs.readFile('/report.txt') === 'x'); +} + +// B. cp over a directory must refuse too +{ + const { vfs, mod, out } = mk(); + vfs.mkdirRecursive('/docs'); + vfs.writeFile('/docs/keep.txt', 'precious'); + vfs.writeFile('/report.txt', 'x'); + out.length = 0; + mod.commands['cp'](['/report.txt', '/docs']); + const msg = out.map(o => o.text).join(' '); + ok('B1 cp onto a directory is refused', /is a directory/.test(msg), msg); + ok('B2 destination children intact', vfs.readFile('/docs/keep.txt') === 'precious'); +} + +// C. mv a a is a no-op, not a delete +{ + const { vfs } = mk(); + vfs.writeFile('/a.txt', 'still here'); + ok('C1 mv onto itself returns true', vfs.rename('/a.txt', '/a.txt') === true); + ok('C2 file survived', vfs.readFile('/a.txt') === 'still here'); +} + +// D. overwriting a file with a file still works (documented mv behaviour) +{ + const { vfs } = mk(); + vfs.writeFile('/one.txt', '1'); + vfs.writeFile('/two.txt', '2'); + ok('D1 mv over a file succeeds', vfs.rename('/one.txt', '/two.txt') === true); + ok('D2 new content in place', vfs.readFile('/two.txt') === '1'); + ok('D3 source gone', vfs.readFile('/one.txt') === null); +} + +// E. ls -l <path> lists that path, not the cwd (MAINT-016) +{ + const { vfs, mod, out } = mk(); + vfs.mkdirRecursive('/work'); + vfs.writeFile('/work/inside.txt', 'x'); + vfs.writeFile('/cwd-only.txt', 'y'); + vfs.currentPath = []; + out.length = 0; + mod.commands['ls'](['-l', '/work']); + const text = out.map(o => o.text).join('\n'); + ok('E1 flag-first form lists the given path', /inside\.txt/.test(text) && !/cwd-only\.txt/.test(text), text.replace(/\n/g, ' | ')); +} + +// F. path-first form keeps working +{ + const { vfs, mod, out } = mk(); + vfs.mkdirRecursive('/work'); + vfs.writeFile('/work/inside.txt', 'x'); + out.length = 0; + mod.commands['ls'](['/work', '-l']); + const text = out.map(o => o.text).join('\n'); + ok('F1 path-first form still lists the path', /inside\.txt/.test(text), text.replace(/\n/g, ' | ')); +} + +if (fails === 0) { + console.log('test-fs-mv-cp-guard: ' + passes + ' passed, 0 failed'); +} else { + console.log('test-fs-mv-cp-guard: ' + passes + ' passed, ' + fails + ' FAILED'); + process.exit(1); +} |