diff --git a/packages/cli/tests/business-imports.mjs b/packages/cli/tests/business-imports.mjs index ddc50223b..e16494171 100644 --- a/packages/cli/tests/business-imports.mjs +++ b/packages/cli/tests/business-imports.mjs @@ -3,21 +3,96 @@ // a plug-in source file (row 333's model-quota.mjs) does not break the release tests the way it did // when each fixture named kinds.mjs alone. A real copy ships all of packages/plugins (scripts/ // stack-copy.sh COPY_PATHS); a fixture models a smaller one and takes only what the loader reads. -import {existsSync,readdirSync,readFileSync} from 'node:fs'; +// +// Vikunja #779: the helper reads imports with a regex, not a parser, so it stays strict about what +// it cannot read. Comments are blanked before matching, so an import in a comment is not one, and an +// import form inside a string or a regular expression is not one either. A dynamic import written +// with a space, `import ('./x.mjs')`, counts. These throw: a dynamic import whose argument is not a +// plain string, a file with an unterminated string, comment, template or regular expression, a +// target that is missing or not a regular file, and a target whose real path leaves the repository. +// The scanner knows line and block comments, quoted strings, template literals, regular expression +// literals (by the usual guess from the previous token) and a hashbang. A template literal with a +// backtick nested inside its ${} is outside that, and so is a regular expression it guesses wrong. +import {readdirSync,readFileSync,realpathSync,statSync} from 'node:fs'; import {dirname,join,relative,resolve,sep} from 'node:path'; -const IMPORT=/(?:import|export)\s[^'"`;]*?\bfrom\s*['"](\.{1,2}\/[^'"]+)['"]|\bimport\s*['"](\.{1,2}\/[^'"]+)['"]|\bimport\(\s*['"](\.{1,2}\/[^'"]+)['"]\s*\)/g; +const IMPORT=/(?:import|export)\s[^'"`;]*?\bfrom\s*['"](\.{1,2}\/[^'"]+)['"]|\bimport\s*['"](\.{1,2}\/[^'"]+)['"]|(?~^'; const sources=(dir)=>readdirSync(dir,{withFileTypes:true}).flatMap((e)=>e.isDirectory()?sources(join(dir,e.name)):e.name.endsWith('.mjs')?[join(dir,e.name)]:[]); +// The text with every comment blanked and every template literal emptied, and the ranges of the +// quoted strings and regular expressions left in it, as [from,to) offsets into the returned code. +// Throws on a string, comment, template or regular expression that never ends. +function scan(text){ + let code='',i=0,previous=''; + const literals=[]; + const fail=(what)=>{throw new Error(`unterminated ${what}`);}; + const keep=(end,last)=>{literals.push([code.length,code.length+end-i]);code+=text.slice(i,end);i=end;previous=last;}; + if(text.startsWith('#!')){const end=text.indexOf('\n');i=end<0?text.length:end;} + while(i=text.length)fail('string'); + keep(j+1,c);continue; + } + if(c==='`'){ + let j=i+1; + for(;j=text.length)fail('template'); + code+='``';i=j+1;previous='`';continue; + } + if(c==='/'&&(previous===''||REGEX_AFTER.includes(previous))){ + let j=i+1,inClass=false; + for(;j=text.length)fail('regular expression'); + keep(j+1,'/');continue; + } + code+=c;i++; + if(!/\s/.test(c))previous=c; + } + return {code,literals}; +} + +// The specifiers a source file imports by a relative path. A dynamic import of anything but a plain +// string throws, as does a file the scanner cannot read to its end. +function specifiers(file,text){ + let scanned; + try{scanned=scan(text);} + catch(error){throw new Error(`${file} cannot be read for imports: ${error.message}`);} + const {code,literals}=scanned,live=(m)=>!literals.some(([from,to])=>m.index>=from&&m.indexm[1]??m[2]??m[3]); +} + // Repo-relative paths, sorted: every file that a relative import from packages/business/src reaches -// outside it, following the imports of those files in turn. An import of a missing file throws. +// outside it, following the imports of those files in turn. An import of a missing file, of anything +// but a regular file, or of a file whose real path is outside the repository throws, wherever the +// importing file sits, so an import of a missing file inside packages/business/src throws too. export function businessImportedFiles(repo){ - const own=join(repo,'packages','business','src')+sep,found=new Set(),todo=sources(own); + const own=join(repo,'packages','business','src')+sep,found=new Set(),checked=new Set(),todo=sources(own),root=realpathSync(repo)+sep; for(let file;(file=todo.pop())!==undefined;){ - for(const m of readFileSync(file,'utf8').matchAll(IMPORT)){ - const spec=m[1]??m[2]??m[3],target=resolve(dirname(file),spec); + for(const spec of specifiers(relative(repo,file),readFileSync(file,'utf8'))){ + const target=resolve(dirname(file),spec),from=`${relative(repo,file)} imports ${spec}`; + if(!checked.has(target)){ + let info,real; + try{info=statSync(target);real=realpathSync(target);} + catch(error){throw error.code==='ENOENT'||error.code==='ENOTDIR'?new Error(`${from}, which does not exist`):error;} + if(!info.isFile())throw new Error(`${from}, which is not a regular file`); + if(!real.startsWith(root))throw new Error(`${from}, which is outside the repository`); + checked.add(target); + } if(target.startsWith(own)||found.has(target))continue; - if(!existsSync(target))throw new Error(`${relative(repo,file)} imports ${spec}, which does not exist`); found.add(target); if(target.endsWith('.mjs'))todo.push(target); } diff --git a/packages/cli/tests/business-imports.test.mjs b/packages/cli/tests/business-imports.test.mjs new file mode 100644 index 000000000..977cee31a --- /dev/null +++ b/packages/cli/tests/business-imports.test.mjs @@ -0,0 +1,154 @@ +// Vikunja #779: the business-imports helper on synthetic trees. Every tree is built under a private +// temp directory with the repository one level down, so a test can put a file outside the repository. +import test, {after} from 'node:test'; +import assert from 'node:assert/strict'; +import {mkdirSync,mkdtempSync,rmSync,symlinkSync,writeFileSync} from 'node:fs'; +import {tmpdir} from 'node:os'; +import {dirname,join} from 'node:path'; +import {fileURLToPath} from 'node:url'; +import {businessImportedFiles} from './business-imports.mjs'; + +const ROOT=fileURLToPath(new URL('../../..',import.meta.url)).replace(/\/$/,''); +const base=mkdtempSync(join(tmpdir(),'bi-')); +after(()=>rmSync(base,{recursive:true,force:true})); +const BIZ='repo/packages/business/src/',EXT='repo/packages/other/src/',EMPTY='export {};\n'; +let count=0; +// files: path under the tree's top directory -> text. links: path -> target path, both under it. +function tree(files,links={}){ + const top=join(base,`t${++count}`); + for(const [path,text] of Object.entries(files)){mkdirSync(dirname(join(top,path)),{recursive:true});writeFileSync(join(top,path),text);} + for(const [path,target] of Object.entries(links)){mkdirSync(dirname(join(top,path)),{recursive:true});symlinkSync(join(top,target),join(top,path));} + return join(top,'repo'); +} +const withBusiness=(text,extra={})=>tree({[`${BIZ}a.mjs`]:text,...extra}); + +test('the real tree gives the plug-in files the loader reads', ()=>{ + const found=businessImportedFiles(ROOT); + assert.ok(found.includes('packages/plugins/src/kinds.mjs'),found.join(' ')); + assert.ok(found.includes('packages/plugins/src/model-quota.mjs'),found.join(' ')); + assert.deepEqual(found,[...found].sort()); +}); + +test('every static form and a literal dynamic import is found, and the imports of what it finds in turn', ()=>{ + const repo=withBusiness([ + "import x from '../../other/src/from.mjs';", + "import {\n y,\n z,\n} from '../../other/src/multi.mjs';", + "import '../../other/src/bare.mjs';", + "export * from '../../other/src/reexport.mjs';", + "export {w} from '../../other/src/named.mjs';", + "const d=await import('../../other/src/dynamic.mjs');", + "const n=await import(\n '../../other/src/newline.mjs'\n);", + ].join('\n'),{ + [`${EXT}from.mjs`]:"import '../../third/src/deep.mjs';\n", + 'repo/packages/third/src/deep.mjs':EMPTY, + ...Object.fromEntries(['multi','bare','reexport','named','dynamic','newline'].map(n=>[`${EXT}${n}.mjs`,EMPTY])), + }); + assert.deepEqual(businessImportedFiles(repo),[ + ...['bare','dynamic','from','multi','named','newline','reexport'].map(n=>`packages/other/src/${n}.mjs`), + 'packages/third/src/deep.mjs', + ]); +}); + +test('#779 a dynamic import written with a space or a line break before the bracket is found', ()=>{ + const repo=withBusiness("const s=await import ('../../other/src/spaced.mjs');\nconst t=await import\n ('../../other/src/broken.mjs');\n", + {[`${EXT}spaced.mjs`]:EMPTY,[`${EXT}broken.mjs`]:EMPTY}); + assert.deepEqual(businessImportedFiles(repo),['packages/other/src/broken.mjs','packages/other/src/spaced.mjs']); +}); + +test('#779 an import in a line or block comment is not an import, and one after a comment is', ()=>{ + const repo=withBusiness([ + "// import '../../other/src/missing1.mjs';", + "/* import x from '../../other/src/missing2.mjs'; */", + "/*\n import('../../other/src/missing3.mjs')\n*/", + "const x=1; // import '../../other/src/missing4.mjs'", + "import '../../other/src/real.mjs'; // import('../../other/src/missing5.mjs')", + "// import(name)", + "/* import(name) */ import '../../other/src/after.mjs';", + ].join('\n'),{[`${EXT}real.mjs`]:EMPTY,[`${EXT}after.mjs`]:EMPTY}); + assert.deepEqual(businessImportedFiles(repo),['packages/other/src/after.mjs','packages/other/src/real.mjs']); +}); + +test('#779 comment markers inside strings, templates and regular expressions do not hide an import', ()=>{ + const repo=withBusiness([ + "const url='http://example.invalid/a'; import '../../other/src/after-url.mjs';", + "const glob=\"/* open\"; import '../../other/src/after-glob.mjs';", + "const tpl=`// not a comment ${1}`; import '../../other/src/after-template.mjs';", + "const re=/['\"]\\/\\/x/; import '../../other/src/after-regex.mjs';", + "const half=a / 2;\nconst mid=(a+b) / 2;\nimport '../../other/src/after-divide.mjs';", + ].join('\n'),Object.fromEntries(['url','glob','template','regex','divide'].map(n=>[`${EXT}after-${n}.mjs`,EMPTY]))); + assert.deepEqual(businessImportedFiles(repo),['url','glob','template','regex','divide'].map(n=>`packages/other/src/after-${n}.mjs`).sort()); +}); + +test('#779 an import form inside a string or a regular expression is not an import', ()=>{ + const repo=withBusiness([ + "const a=\"import '../../other/src/missing1.mjs'\";", + "const b='import(name)';", + "const c=/import (name)/;", + "const d=\"export {x} from '../../other/src/missing2.mjs'\";", + "import '../../other/src/real.mjs';", + ].join('\n'),{[`${EXT}real.mjs`]:EMPTY}); + assert.deepEqual(businessImportedFiles(repo),['packages/other/src/real.mjs']); +}); + +test('#779 a hashbang line is skipped and a bare slash in it does not start a regular expression', ()=>{ + const repo=withBusiness("#!/usr/bin/env node\nimport '../../other/src/real.mjs';\n",{[`${EXT}real.mjs`]:EMPTY}); + assert.deepEqual(businessImportedFiles(repo),['packages/other/src/real.mjs']); + const bare=withBusiness("#!/odd\nimport '../../other/src/real.mjs';\n",{[`${EXT}real.mjs`]:EMPTY}); + assert.deepEqual(businessImportedFiles(bare),['packages/other/src/real.mjs']); +}); + +test('#779 a dynamic import of anything but a plain string throws, and what only looks like one does not', ()=>{ + for(const argument of ['name',"'../../other/src/' + name",'`../../other/src/${name}.mjs`',"'../../other/src/a.mjs', {with:{type:'json'}}","'../../other/src/a.mjs' + ''"]){ + const repo=withBusiness(`const m=await import(${argument});\n`,{[`${EXT}a.mjs`]:EMPTY}); + assert.throws(()=>businessImportedFiles(repo),/business\/src\/a\.mjs has a dynamic import that is not a plain string literal/,argument); + } + const repo=withBusiness([ + 'const u=import.meta.url;','loader.import(thing);','reimport(thing);','$import(thing);', + "const fs=await import('node:fs');","const s='import(name)';", + ].join('\n')); + assert.deepEqual(businessImportedFiles(repo),[]); +}); + +test('#779 a file the scanner cannot read to its end throws and names the file, business or external', ()=>{ + for(const [what,text] of [['string',"const a='open;\n"],['string','const a="open'],['comment',"/* open\nimport '../../other/src/a.mjs';\n"],['template','const a=`open'],['regular expression','const a=/open\n']]){ + const repo=withBusiness(text,{[`${EXT}a.mjs`]:EMPTY}); + assert.throws(()=>businessImportedFiles(repo),new RegExp(`packages/business/src/a\\.mjs cannot be read for imports: unterminated ${what}`),text); + } + const repo=withBusiness("import '../../other/src/a.mjs';\n",{[`${EXT}a.mjs`]:"/* never closed\n"}); + assert.throws(()=>businessImportedFiles(repo),/packages\/other\/src\/a\.mjs cannot be read for imports: unterminated comment/); +}); + +test('#779 an import of a file that is missing throws, inside packages/business/src or out of it', ()=>{ + const inside=withBusiness("import './missing.mjs';\n"); + assert.throws(()=>businessImportedFiles(inside),/packages\/business\/src\/a\.mjs imports \.\/missing\.mjs, which does not exist/); + const outside=withBusiness("import '../../other/src/missing.mjs';\n"); + assert.throws(()=>businessImportedFiles(outside),/imports \.\.\/\.\.\/other\/src\/missing\.mjs, which does not exist/); + const broken=tree({[`${BIZ}a.mjs`]:"import '../../other/src/link.mjs';\n"},{[`${EXT}link.mjs`]:'gone.mjs'}); + assert.throws(()=>businessImportedFiles(broken),/which does not exist/); + const beyond=withBusiness("import '../../other/src/a.mjs/b.mjs';\n",{[`${EXT}a.mjs`]:EMPTY}); + assert.throws(()=>businessImportedFiles(beyond),/which does not exist/); +}); + +test('#779 an import of a directory throws', ()=>{ + const repo=withBusiness("import '../../other/src/dir';\n",{[`${EXT}dir/index.mjs`]:EMPTY}); + assert.throws(()=>businessImportedFiles(repo),/imports \.\.\/\.\.\/other\/src\/dir, which is not a regular file/); +}); + +test('#779 a target outside the repository throws, by a path that climbs out or by a symlink', ()=>{ + const climbs=tree({[`${BIZ}a.mjs`]:"import '../../../../outside.mjs';\n",'outside.mjs':EMPTY}); + assert.throws(()=>businessImportedFiles(climbs),/imports \.\.\/\.\.\/\.\.\/\.\.\/outside\.mjs, which is outside the repository/); + const sibling=tree({[`${BIZ}a.mjs`]:"import '../../../../repo2/x.mjs';\n",'repo2/x.mjs':EMPTY}); + assert.throws(()=>businessImportedFiles(sibling),/which is outside the repository/); + const linked=tree({[`${BIZ}a.mjs`]:"import '../../other/src/link.mjs';\n",'outside.mjs':EMPTY},{[`${EXT}link.mjs`]:'outside.mjs'}); + assert.throws(()=>businessImportedFiles(linked),/imports \.\.\/\.\.\/other\/src\/link\.mjs, which is outside the repository/); + const folder=tree({[`${BIZ}a.mjs`]:"import '../../other/src/out/x.mjs';\n",'outside/x.mjs':EMPTY},{[`${EXT}out`]:'outside'}); + assert.throws(()=>businessImportedFiles(folder),/which is outside the repository/); +}); + +test('#779 a symlink that stays inside the repository, and a repository reached by a symlink, are fine', ()=>{ + const repo=tree({[`${BIZ}a.mjs`]:"import '../../other/src/link.mjs';\n",'repo/packages/third/real.mjs':EMPTY},{[`${EXT}link.mjs`]:'repo/packages/third/real.mjs'}); + assert.deepEqual(businessImportedFiles(repo),['packages/other/src/link.mjs']); + const via=join(base,`via${++count}`); + symlinkSync(repo,via); + assert.deepEqual(businessImportedFiles(via),['packages/other/src/link.mjs']); +});