From 8eb52f6521d554f677552d924649abe682f475c4 Mon Sep 17 00:00:00 2001 From: Trygve Lie <173003+trygve-lie@users.noreply.github.com> Date: Fri, 8 May 2020 08:55:16 +0200 Subject: [PATCH] Improve error handling on upload (#126) * Improve error handling on upload * Lint love --- examples/pkg.alias.put.js | 4 +- examples/pkg.put.js | 4 +- fixtures/package.tar | Bin 0 -> 10240 bytes fixtures/package.tar.bz2 | Bin 0 -> 806 bytes fixtures/package.tar.gz | Bin 0 -> 693 bytes lib/classes/asset.js | 2 +- lib/handlers/alias.delete.js | 20 ++--- lib/handlers/alias.get.js | 20 ++--- lib/handlers/alias.post.js | 20 ++--- lib/handlers/alias.put.js | 20 ++--- lib/handlers/map.get.js | 20 ++--- lib/handlers/map.put.js | 20 ++--- lib/handlers/pkg.get.js | 20 ++--- lib/handlers/pkg.log.js | 20 ++--- lib/handlers/pkg.put.js | 99 ++++++++++++++--------- lib/handlers/versions.get.js | 19 +++-- lib/sinks/fs.js | 54 +++++++++---- lib/sinks/mem.js | 59 +++++++++----- lib/sinks/test.js | 41 ++++++++-- test/classes/asset.js | 2 +- test/handlers/pkg.put.js | 152 +++++++++++++++++++++++++++++++++++ test/sinks/fs.js | 71 +++++++++++----- test/sinks/mem.js | 67 ++++++++++----- test/sinks/test.js | 67 ++++++++++----- 24 files changed, 565 insertions(+), 236 deletions(-) create mode 100644 fixtures/package.tar create mode 100644 fixtures/package.tar.bz2 create mode 100644 fixtures/package.tar.gz create mode 100644 test/handlers/pkg.put.js diff --git a/examples/pkg.alias.put.js b/examples/pkg.alias.put.js index f7795257..199831d3 100644 --- a/examples/pkg.alias.put.js +++ b/examples/pkg.alias.put.js @@ -23,11 +23,11 @@ const put = async (address) => { const auth = await authenticate(address); const formData = new FormData(); - formData.append('version', '8.4.1'); + formData.append('version', '1.2.1'); const headers = {'Authorization': `Bearer ${auth.token}`, ...formData.getHeaders()}; - const res = await fetch(`${address}/pkg/fuzz/v8`, { + const res = await fetch(`${address}/npm/lit-html/v1`, { method: 'PUT', body: formData, headers, diff --git a/examples/pkg.put.js b/examples/pkg.put.js index 9f960861..9f91db53 100644 --- a/examples/pkg.put.js +++ b/examples/pkg.put.js @@ -24,11 +24,11 @@ const put = async (address) => { const auth = await authenticate(address); const formData = new FormData(); - formData.append('package', fs.createReadStream('../fixtures/archive.tgz')); + formData.append('package', fs.createReadStream('./lit-html.tar')); const headers = {'Authorization': `Bearer ${auth.token}`, ...formData.getHeaders()}; - const res = await fetch(`${address}/pkg/fuzz/8.4.1`, { + const res = await fetch(`${address}/npm/lit-html/1.2.1`, { method: 'PUT', body: formData, headers, diff --git a/fixtures/package.tar b/fixtures/package.tar new file mode 100644 index 0000000000000000000000000000000000000000..39cbfb7e56de2e6657ee65cf7c508a48c35670bf GIT binary patch literal 10240 zcmeHL-*4J55azjmh37uBbZHVNagabfq-d+AuBuk4PwO5`aH^~!X0hoHA^!KBY1tAA zqgorvG+Rg^=kxjWoxk&4Qcj4|sCT$9T9h{6L0Z%;KlPLz4dRD1prP*((r^j!0@^^d zUbh~skELP-LJd{iU)}K4JFc5wy>hE(QXT&^AMiW-yR6Am9pEqs%5k5c13l#u+Jg8~ z8itevUJK&ywLFNwTa!N(v;TSg-zDQbR;kGMn-e^i97$EAvBI4jV1?)=LUc*z77mEG zwOPgswJ(}h-n_g$0u}!2 z5Y^z4=J=oZA40OP^53VfZ}?wBD4v*{;2T~@;6HBr7EaQP<2IgC7F+m%&Jwr3V%v6> z{w^d~a3E{kvh12WvWT68UGu|{7-U>IOiHe#1DOPF7_hk11_+;LM#e=tQc|1x_RG7T z$Q93(c9P$XM4`}tCv2Q41iPXxBAv?|c20-0IW0>HF+_W|_G%9%FY{RBQXxoXpuPtW zEv=+E&}=TsKu^oGCpjDP8A_lBTRkj=KeL>v6@XegBZ3Zoe-uSLvOY!8sr8|ETpabR zvnV?2S*IZRXdOqr{%^_06Fhwqwbv62S(s6YPA>5tjqtz z^Go?Zg!<1XF2vu|e=malS&Ma<$Cj0OY-yQiYP@Hm`tK-J`2SY*A0>5g--@^kLvU4ahj k{&uk7u6Z>G7z7Lg1_6VBLBJqj5HJWB1PlTOfxkrHCt}@CNdN!< literal 0 HcmV?d00001 diff --git a/fixtures/package.tar.bz2 b/fixtures/package.tar.bz2 new file mode 100644 index 0000000000000000000000000000000000000000..facd86705fa7f9af8c4fd70c70d2add378db8b3e GIT binary patch literal 806 zcmV+>1KIpST4*^jL0KkKS#+j<*Z>0(|J1~g0RVXa|C&3mx}JZ(|LOn%01yB|00>Y6 zUI6F-4min^Mj@t#0%&4r(OtuM z27mwn000002N^PG#5B;rO$<#MWW)@NCXF=0445iO0uiM%CK{fRspOuf2w|z}WZI3S zJp?@yYI=w9I;UH+dA{LMbjV?_xR~c6OttH-9`*O~HbVrx-*IJ1tk+tvyKRusA>0)r zSuIVsYmI=y9|2CU2UC@wB(j-3>iCN-#XG%ym$p03x2x=3G(TsNbl8|nG|DSFmoAlq z!pd4wn93!-g`b!e&{MAX(=yf%j z%_N-Q=5Y_BijLxzQAy?1!=32RNlG=I^32SpWmz_jN%)806PKm!>3EvN#5Qa!7R}9N zuC)C*IoneO_*l#-l3jhlwDT>TQ40yI1l%yT)x{Nv6aWZ9qXAHg5rC+e1%aV~XAJns zg)Ur|uuQJSa~+h}SumHT1y&)8$M&UBITxwMr9Kzl+Bn`?_fHdfMo)9f?v?HH*ST{f zdwTa7jGq_s$mO4-Lgk*7IhEp}+}dqCj?&-1Bu^vW+i|yJ{JM&4D4&5yrr=}qpD~o< zbWWD3k(y>k4(h7blwM*@w+5q?Z|PUg*-RVibT53d?oGu`pw%6k{<%#q7}ILgODSi8 z9~ACQTQ2c4o96?DT#nwN$9eTrujlXH?9!XFJlTKKM5$JbFO=Aq5?=ddOX7zWCl>MJ zZZ}!WJadI6`y&>pZ=<%Cr`~^#Rq3+kvK^;GLAwtB8Mf`YoDyd#7=~`dtR?Admc4bw zS-4X9ouiz@#&At(iKafo!prv5HI5HNT&)C4wTRP0DU5hd;kCd literal 0 HcmV?d00001 diff --git a/fixtures/package.tar.gz b/fixtures/package.tar.gz new file mode 100644 index 0000000000000000000000000000000000000000..6cea57663bde79f9ae40014de6c24477f812c8ab GIT binary patch literal 693 zcmV;m0!sZKiwFRZ>9Sq`1MQeyZ`v>vhPm#q@Z5`*E+ugi2MNSQ8m(2;Rn;o>YTbni zPL(yp%r^ZH;=k`0+Kn(OF1DoI=0`{d=U|&0zlS%@IU#PVUOb>nscA`L=5;NYD$OK9EiE5!?)b}b+phnmzcN$;+Fo%a4QS&D9yR$xTGI5^7-j(hldKVSzh<*ab;GwyWUjjt;O zBe+mC?pblmT_saDW4HWpB1Q$*E>ntYw)%=vh6&GcnbjL}}ltZEM~UYDsTl}1p?AbbxR(_1NX5ZPYu zgE-S?R#LL@@-9IfOtlPyH;X_MFkV(fh{5lV;+V(IX&j$BACeP!lsFf0e33Zk&~xmZ z#L4hy&+!?a*O>n?%gZdeY?7Box3|V@6BgM2F9j2lX zvWIrFECXT9jkfB1`mt2?U(Xy{{l7=n&wqSs&wm=hXmR$70FD0UJMO`YR{!b00v*(S bc3`nsEEbE!VzF2(mOsOHxy-Ui04M+eVVz;W literal 0 HcmV?d00001 diff --git a/lib/classes/asset.js b/lib/classes/asset.js index 3243d589..bd629e6d 100644 --- a/lib/classes/asset.js +++ b/lib/classes/asset.js @@ -21,7 +21,7 @@ const Asset = class Asset { type = '', org = '', } = {}) { - this._mimeType = mime.getType(pathname) || ''; + this._mimeType = mime.getType(pathname) || 'application/octet-stream'; this._type = type.toLowerCase(); this._size = -1; diff --git a/lib/handlers/alias.delete.js b/lib/handlers/alias.delete.js index dfac2b3c..6d985117 100644 --- a/lib/handlers/alias.delete.js +++ b/lib/handlers/alias.delete.js @@ -49,16 +49,6 @@ const AliasDel = class AliasDel { async handler(req, user, type, name, alias) { const end = this._histogram.timer(); - const url = originalUrl(req); - const org = this._orgRegistry.get(url.hostname); - - if (!org) { - this._log.info(`alias:del - Hostname does not match a configured organization - ${url.hostname}`); - const e = new HttpError.InternalServerError(); - end({ labels: { success: false, status: e.status } }); - throw e; - } - try { validators.alias(alias); validators.name(name); @@ -70,6 +60,16 @@ const AliasDel = class AliasDel { throw e; } + const url = originalUrl(req); + const org = this._orgRegistry.get(url.hostname); + + if (!org) { + this._log.info(`alias:del - Hostname does not match a configured organization - ${url.hostname}`); + const e = new HttpError.InternalServerError(); + end({ labels: { success: false, status: e.status } }); + throw e; + } + const path = createFilePathToAlias({ org, type, name, alias }); const exist = await this._exist(path); if (!exist) { diff --git a/lib/handlers/alias.get.js b/lib/handlers/alias.get.js index 510ff373..6e3d371e 100644 --- a/lib/handlers/alias.get.js +++ b/lib/handlers/alias.get.js @@ -42,16 +42,6 @@ const AliasGet = class AliasGet { async handler(req, type, name, alias, extra) { const end = this._histogram.timer(); - const url = originalUrl(req); - const org = this._orgRegistry.get(url.hostname); - - if (!org) { - this._log.info(`alias:get - Hostname does not match a configured organization - ${url.hostname}`); - const e = new HttpError.InternalServerError(); - end({ labels: { success: false, status: e.status } }); - throw e; - } - try { validators.alias(alias); validators.extra(extra); @@ -64,6 +54,16 @@ const AliasGet = class AliasGet { throw e; } + const url = originalUrl(req); + const org = this._orgRegistry.get(url.hostname); + + if (!org) { + this._log.info(`alias:get - Hostname does not match a configured organization - ${url.hostname}`); + const e = new HttpError.InternalServerError(); + end({ labels: { success: false, status: e.status } }); + throw e; + } + const path = createFilePathToAlias({ org, type, name, alias }); try { diff --git a/lib/handlers/alias.post.js b/lib/handlers/alias.post.js index 80f89c2b..386921e3 100644 --- a/lib/handlers/alias.post.js +++ b/lib/handlers/alias.post.js @@ -139,16 +139,6 @@ const AliasPost = class AliasPost { async handler(req, user, type, name, alias) { const end = this._histogram.timer(); - const url = originalUrl(req); - const org = this._orgRegistry.get(url.hostname); - - if (!org) { - this._log.info(`alias:post - Hostname does not match a configured organization - ${url.hostname}`); - const e = new HttpError.InternalServerError(); - end({ labels: { success: false, status: e.status } }); - throw e; - } - try { validators.alias(alias); validators.name(name); @@ -160,6 +150,16 @@ const AliasPost = class AliasPost { throw e; } + const url = originalUrl(req); + const org = this._orgRegistry.get(url.hostname); + + if (!org) { + this._log.info(`alias:post - Hostname does not match a configured organization - ${url.hostname}`); + const e = new HttpError.InternalServerError(); + end({ labels: { success: false, status: e.status } }); + throw e; + } + const author = new Author(user); const incoming = new HttpIncoming(req, { diff --git a/lib/handlers/alias.put.js b/lib/handlers/alias.put.js index db78c565..4bc47978 100644 --- a/lib/handlers/alias.put.js +++ b/lib/handlers/alias.put.js @@ -139,16 +139,6 @@ const AliasPut = class AliasPut { async handler(req, user, type, name, alias) { const end = this._histogram.timer(); - const url = originalUrl(req); - const org = this._orgRegistry.get(url.hostname); - - if (!org) { - this._log.info(`alias:put - Hostname does not match a configured organization - ${url.hostname}`); - const e = new HttpError.InternalServerError(); - end({ labels: { success: false, status: e.status } }); - throw e; - } - try { validators.alias(alias); validators.name(name); @@ -160,6 +150,16 @@ const AliasPut = class AliasPut { throw e; } + const url = originalUrl(req); + const org = this._orgRegistry.get(url.hostname); + + if (!org) { + this._log.info(`alias:put - Hostname does not match a configured organization - ${url.hostname}`); + const e = new HttpError.InternalServerError(); + end({ labels: { success: false, status: e.status } }); + throw e; + } + const author = new Author(user); const incoming = new HttpIncoming(req, { diff --git a/lib/handlers/map.get.js b/lib/handlers/map.get.js index 66a774ad..85f37cae 100644 --- a/lib/handlers/map.get.js +++ b/lib/handlers/map.get.js @@ -43,16 +43,6 @@ const MapGet = class MapGet { async handler(req, name, version) { const end = this._histogram.timer(); - const url = originalUrl(req); - const org = this._orgRegistry.get(url.hostname); - - if (!org) { - this._log.info(`map:get - Hostname does not match a configured organization - ${url.hostname}`); - const e = new HttpError.InternalServerError(); - end({ labels: { success: false, status: e.status } }); - throw e; - } - try { validators.version(version); validators.name(name); @@ -63,6 +53,16 @@ const MapGet = class MapGet { throw e; } + const url = originalUrl(req); + const org = this._orgRegistry.get(url.hostname); + + if (!org) { + this._log.info(`map:get - Hostname does not match a configured organization - ${url.hostname}`); + const e = new HttpError.InternalServerError(); + end({ labels: { success: false, status: e.status } }); + throw e; + } + const path = createFilePathToImportMap({ org, name, version }); try { diff --git a/lib/handlers/map.put.js b/lib/handlers/map.put.js index 9cc4286e..744711db 100644 --- a/lib/handlers/map.put.js +++ b/lib/handlers/map.put.js @@ -161,16 +161,6 @@ const MapPut = class MapPut { async handler(req, user, name, version) { const end = this._histogram.timer(); - const url = originalUrl(req); - const org = this._orgRegistry.get(url.hostname); - - if (!org) { - this._log.info(`map:put - Hostname does not match a configured organization - ${url.hostname}`); - const e = new HttpError.InternalServerError(); - end({ labels: { success: false, status: e.status } }); - throw e; - } - try { validators.version(version); validators.name(name); @@ -181,6 +171,16 @@ const MapPut = class MapPut { throw e; } + const url = originalUrl(req); + const org = this._orgRegistry.get(url.hostname); + + if (!org) { + this._log.info(`map:put - Hostname does not match a configured organization - ${url.hostname}`); + const e = new HttpError.InternalServerError(); + end({ labels: { success: false, status: e.status } }); + throw e; + } + const author = new Author(user); const incoming = new HttpIncoming(req, { diff --git a/lib/handlers/pkg.get.js b/lib/handlers/pkg.get.js index 16c140d4..ee71ddb1 100644 --- a/lib/handlers/pkg.get.js +++ b/lib/handlers/pkg.get.js @@ -44,16 +44,6 @@ const PkgGet = class PkgGet { async handler(req, type, name, version, extra) { const end = this._histogram.timer(); - const url = originalUrl(req); - const org = this._orgRegistry.get(url.hostname); - - if (!org) { - this._log.info(`pkg:get - Hostname does not match a configured organization - ${url.hostname}`); - const e = new HttpError.InternalServerError(); - end({ labels: { success: false, status: e.status } }); - throw e; - } - try { validators.version(version); validators.extra(extra); @@ -66,6 +56,16 @@ const PkgGet = class PkgGet { throw e; } + const url = originalUrl(req); + const org = this._orgRegistry.get(url.hostname); + + if (!org) { + this._log.info(`pkg:get - Hostname does not match a configured organization - ${url.hostname}`); + const e = new HttpError.InternalServerError(); + end({ labels: { success: false, status: e.status } }); + throw e; + } + const asset = new Asset({ pathname: extra, version, diff --git a/lib/handlers/pkg.log.js b/lib/handlers/pkg.log.js index 3055cb84..43ed7257 100644 --- a/lib/handlers/pkg.log.js +++ b/lib/handlers/pkg.log.js @@ -42,16 +42,6 @@ const PkgLog = class PkgLog { async handler(req, type, name, version) { const end = this._histogram.timer(); - const url = originalUrl(req); - const org = this._orgRegistry.get(url.hostname); - - if (!org) { - this._log.info(`pkg:log - Hostname does not match a configured organization - ${url.hostname}`); - const e = new HttpError.InternalServerError(); - end({ labels: { success: false, status: e.status } }); - throw e; - } - try { validators.version(version); validators.name(name); @@ -63,6 +53,16 @@ const PkgLog = class PkgLog { throw e; } + const url = originalUrl(req); + const org = this._orgRegistry.get(url.hostname); + + if (!org) { + this._log.info(`pkg:log - Hostname does not match a configured organization - ${url.hostname}`); + const e = new HttpError.InternalServerError(); + end({ labels: { success: false, status: e.status } }); + throw e; + } + const path = createFilePathToPackage({ org, type, name, version }); try { diff --git a/lib/handlers/pkg.put.js b/lib/handlers/pkg.put.js index fd1070c0..e91218c2 100644 --- a/lib/handlers/pkg.put.js +++ b/lib/handlers/pkg.put.js @@ -163,8 +163,16 @@ const PkgPut = class PkgPut { `pkg:put - Start extracting package - Field: ${fieldname} - Filename: ${filename}`, ); + file.on('limit', () => { + this._log.info( + `pkg:put - Uploaded package exceeds legal file size limit - Field: ${fieldname} - Filename: ${filename}`, + ); + file.emit('error', new HttpError.PayloadTooLarge()); + }); + const extract = new tar.Parse({ - onentry: entry => { + strict: true, + onentry: (entry) => { // Entries not supported must be thrown // away for extraction to continue if (entry.type.toLowerCase() !== 'file') { @@ -182,10 +190,22 @@ const PkgPut = class PkgPut { `pkg:put - Failed extracting package - Field: ${fieldname} - Filename: ${filename}`, ); this._log.trace(error); - reject(new HttpError.UnsupportedMediaType()); - return; + switch (error.code) { + case 'TAR_BAD_ARCHIVE': + case 'TAR_ABORT': + reject(new HttpError.UnsupportedMediaType()); + break; + case 'TAR_ENTRY_UNSUPPORTED': + case 'TAR_ENTRY_INVALID': + case 'TAR_ENTRY_ERROR': + reject(new HttpError.UnprocessableEntity()); + break; + default: + reject(error); + } + } else { + resolve(files); } - resolve(files); }); }); } @@ -208,34 +228,39 @@ const PkgPut = class PkgPut { `pkg:put - Start writing asset to sink - Pathname: ${path}`, ); - const writer = await this._sink.write( - path, - asset.mimeType, - ); + try { + const writer = await this._sink.write( + path, + asset.mimeType, + ); - entry.on('readable', () => { - entry.read(); - }); + entry.on('readable', () => { + entry.read(); + }); - const hash = new Hash(); + const hash = new Hash(); - pipeline(entry, hash, writer, error => { - if (error) { - this._log.error( - `pkg:put - Failed writing asset to sink - Pathname: ${path}`, - ); - this._log.trace(error); - reject(error); - return; - } + pipeline(entry, hash, writer, error => { + if (error) { + this._log.error( + `pkg:put - Failed writing asset to sink - Pathname: ${path}`, + ); + this._log.trace(error); + reject(error); + return; + } - asset.integrity = hash.hash; + asset.integrity = hash.hash; - this._log.info( - `pkg:put - Successfully wrote asset to sink - Pathname: ${path}`, - ); - resolve(asset); - }); + this._log.info( + `pkg:put - Successfully wrote asset to sink - Pathname: ${path}`, + ); + resolve(asset); + }); + } catch(error) { + reject(error); + + } }); } @@ -269,16 +294,6 @@ const PkgPut = class PkgPut { async handler(req, user, type, name, version) { const end = this._histogram.timer(); - const url = originalUrl(req); - const org = this._orgRegistry.get(url.hostname); - - if (!org) { - this._log.info(`pkg:put - Hostname does not match a configured organization - ${url.hostname}`); - const e = new HttpError.InternalServerError(); - end({ labels: { success: false, status: e.status } }); - throw e; - } - try { validators.version(version); validators.name(name); @@ -290,6 +305,16 @@ const PkgPut = class PkgPut { throw e; } + const url = originalUrl(req); + const org = this._orgRegistry.get(url.hostname); + + if (!org) { + this._log.info(`pkg:put - Hostname does not match a configured organization - ${url.hostname}`); + const e = new HttpError.InternalServerError(); + end({ labels: { success: false, status: e.status } }); + throw e; + } + const author = new Author(user); const incoming = new HttpIncoming(req, { diff --git a/lib/handlers/versions.get.js b/lib/handlers/versions.get.js index 56159732..2b2234f1 100644 --- a/lib/handlers/versions.get.js +++ b/lib/handlers/versions.get.js @@ -43,16 +43,6 @@ const VersionsGet = class VersionsGet { async handler(req, type, name) { const end = this._histogram.timer(); - const url = originalUrl(req); - const org = this._orgRegistry.get(url.hostname); - - if (!org) { - this._log.info(`pkg:latest - Hostname does not match a configured organization - ${url.hostname}`); - const e = new HttpError.InternalServerError(); - end({ labels: { success: false, status: e.status } }); - throw e; - } - try { validators.name(name); validators.type(type); @@ -65,6 +55,15 @@ const VersionsGet = class VersionsGet { throw e; } + const url = originalUrl(req); + const org = this._orgRegistry.get(url.hostname); + + if (!org) { + this._log.info(`pkg:latest - Hostname does not match a configured organization - ${url.hostname}`); + const e = new HttpError.InternalServerError(); + end({ labels: { success: false, status: e.status } }); + throw e; + } const path = createFilePathToVersion({ org, type, name }); try { diff --git a/lib/sinks/fs.js b/lib/sinks/fs.js index be58512d..153f6d5b 100644 --- a/lib/sinks/fs.js +++ b/lib/sinks/fs.js @@ -23,14 +23,19 @@ const SinkFS = class SinkFS extends Sink { } write(filePath, contentType) { - const file = super.constructor.validateFilePath(filePath); - super.constructor.validateContentType(contentType); - return new Promise((resolve, reject) => { - const pathname = path.join(this._config.sinkFsRootPath, file); + try { + super.constructor.validateFilePath(filePath); + super.constructor.validateContentType(contentType); + } catch (error) { + reject(error); + return; + } + + const pathname = path.join(this._config.sinkFsRootPath, filePath); if (pathname.indexOf(this._config.sinkFsRootPath) !== 0) { - reject(new Error(`Directory traversal - ${file}`)); + reject(new Error(`Directory traversal - ${filePath}`)); return; } @@ -61,13 +66,18 @@ const SinkFS = class SinkFS extends Sink { } read(filePath) { - const file = super.constructor.validateFilePath(filePath); - return new Promise((resolve, reject) => { - const pathname = path.join(this._config.sinkFsRootPath, file); + try { + super.constructor.validateFilePath(filePath); + } catch (error) { + reject(error); + return; + } + + const pathname = path.join(this._config.sinkFsRootPath, filePath); if (pathname.indexOf(this._config.sinkFsRootPath) !== 0) { - reject(new Error(`Directory traversal - ${file}`)); + reject(new Error(`Directory traversal - ${filePath}`)); return; } @@ -117,13 +127,18 @@ const SinkFS = class SinkFS extends Sink { } delete(filePath) { - const file = super.constructor.validateFilePath(filePath); - return new Promise((resolve, reject) => { - const pathname = path.join(this._config.sinkFsRootPath, file); + try { + super.constructor.validateFilePath(filePath); + } catch (error) { + reject(error); + return; + } + + const pathname = path.join(this._config.sinkFsRootPath, filePath); if (pathname.indexOf(this._config.sinkFsRootPath) !== 0) { - reject(new Error(`Directory traversal - ${file}`)); + reject(new Error(`Directory traversal - ${filePath}`)); return; } @@ -138,13 +153,18 @@ const SinkFS = class SinkFS extends Sink { } exist(filePath) { - const file = super.constructor.validateFilePath(filePath); - return new Promise((resolve, reject) => { - const pathname = path.join(this._config.sinkFsRootPath, file); + try { + super.constructor.validateFilePath(filePath); + } catch (error) { + reject(error); + return; + } + + const pathname = path.join(this._config.sinkFsRootPath, filePath); if (pathname.indexOf(this._config.sinkFsRootPath) !== 0) { - reject(new Error(`Directory traversal - ${file}`)); + reject(new Error(`Directory traversal - ${filePath}`)); return; } diff --git a/lib/sinks/mem.js b/lib/sinks/mem.js index eff77441..b45f6f48 100644 --- a/lib/sinks/mem.js +++ b/lib/sinks/mem.js @@ -23,14 +23,19 @@ const SinkMem = class SinkMem extends Sink { } write(filePath, contentType) { - const file = super.constructor.validateFilePath(filePath); - const mimeType = super.constructor.validateContentType(contentType); - return new Promise((resolve, reject) => { - const pathname = join(this._rootPath, file); + try { + super.constructor.validateFilePath(filePath); + super.constructor.validateContentType(contentType); + } catch (error) { + reject(error); + return; + } + + const pathname = join(this._rootPath, filePath); if (pathname.indexOf(this._rootPath) !== 0) { - reject(new Error(`Directory traversal - ${file}`)); + reject(new Error(`Directory traversal - ${filePath}`)); return; } @@ -43,7 +48,10 @@ const SinkMem = class SinkMem extends Sink { }); stream.on('finish', () => { - const entry = new Entry({ mimeType, payload }); + const entry = new Entry({ + mimeType: contentType, + payload + }); this._state.set(pathname, entry); }); @@ -52,13 +60,18 @@ const SinkMem = class SinkMem extends Sink { } read(filePath) { - const file = super.constructor.validateFilePath(filePath); - return new Promise((resolve, reject) => { - const pathname = join(this._rootPath, file); + try { + super.constructor.validateFilePath(filePath); + } catch (error) { + reject(error); + return; + } + + const pathname = join(this._rootPath, filePath); if (pathname.indexOf(this._rootPath) !== 0) { - reject(new Error(`Directory traversal - ${file}`)); + reject(new Error(`Directory traversal - ${filePath}`)); return; } @@ -83,13 +96,18 @@ const SinkMem = class SinkMem extends Sink { } delete(filePath) { - const file = super.constructor.validateFilePath(filePath); - return new Promise((resolve, reject) => { - const pathname = join(this._rootPath, file); + try { + super.constructor.validateFilePath(filePath); + } catch (error) { + reject(error); + return; + } + + const pathname = join(this._rootPath, filePath); if (pathname.indexOf(this._rootPath) !== 0) { - reject(new Error(`Directory traversal - ${file}`)); + reject(new Error(`Directory traversal - ${filePath}`)); return; } @@ -105,13 +123,18 @@ const SinkMem = class SinkMem extends Sink { } exist(filePath) { - const file = super.constructor.validateFilePath(filePath); - return new Promise((resolve, reject) => { - const pathname = join(this._rootPath, file); + try { + super.constructor.validateFilePath(filePath); + } catch (error) { + reject(error); + return; + } + + const pathname = join(this._rootPath, filePath); if (pathname.indexOf(this._rootPath) !== 0) { - reject(new Error(`Directory traversal - ${file}`)); + reject(new Error(`Directory traversal - ${filePath}`)); return; } diff --git a/lib/sinks/test.js b/lib/sinks/test.js index 21dd28dc..b610d2bb 100644 --- a/lib/sinks/test.js +++ b/lib/sinks/test.js @@ -72,14 +72,19 @@ const SinkTest = class SinkTest extends Sink { // Common SINK API write(filePath, contentType) { - const file = super.constructor.validateFilePath(filePath); - const mimeType = super.constructor.validateContentType(contentType); - return new Promise((resolve, reject) => { - const pathname = path.join(this._rootPath, file); + try { + super.constructor.validateFilePath(filePath); + super.constructor.validateContentType(contentType); + } catch (error) { + reject(error); + return; + } + + const pathname = path.join(this._rootPath, filePath); if (pathname.indexOf(this._rootPath) !== 0) { - reject(new Error(`Directory traversal - ${file}`)); + reject(new Error(`Directory traversal - ${filePath}`)); return; } @@ -104,7 +109,10 @@ const SinkTest = class SinkTest extends Sink { }); stream.on('finish', () => { - const entry = new Entry({ mimeType, payload, }); + const entry = new Entry({ + mimeType: contentType, + payload + }); this._state.set(pathname, entry); }); @@ -121,6 +129,13 @@ const SinkTest = class SinkTest extends Sink { read(filePath) { return new Promise((resolve, reject) => { + try { + super.constructor.validateFilePath(filePath); + } catch (error) { + reject(error); + return; + } + const pathname = path.join(this._rootPath, filePath); if (pathname.indexOf(this._rootPath) !== 0) { @@ -152,6 +167,13 @@ const SinkTest = class SinkTest extends Sink { delete(filePath) { return new Promise((resolve, reject) => { + try { + super.constructor.validateFilePath(filePath); + } catch (error) { + reject(error); + return; + } + const pathname = path.join(this._rootPath, filePath); if (pathname.indexOf(this._rootPath) !== 0) { @@ -172,6 +194,13 @@ const SinkTest = class SinkTest extends Sink { exist(filePath) { return new Promise((resolve, reject) => { + try { + super.constructor.validateFilePath(filePath); + } catch (error) { + reject(error); + return; + } + const pathname = path.join(this._rootPath, filePath); if (pathname.indexOf(this._rootPath) !== 0) { diff --git a/test/classes/asset.js b/test/classes/asset.js index 208c9b1b..8fc13528 100644 --- a/test/classes/asset.js +++ b/test/classes/asset.js @@ -13,7 +13,7 @@ test('Asset() - Default property values', (t) => { const obj = new Asset(); t.equal(obj.integrity, '', '.integrity should be empty String'); t.equal(obj.pathname, '/', '.pathname should be "/"'); - t.equal(obj.mimeType, '', '.mimetype should be empty String'); + t.equal(obj.mimeType, 'application/octet-stream', '.mimetype should be "application/octet-stream"'); t.equal(obj.version, '', '.version should be empty String'); t.equal(obj.asset, '/', '.asset should be "/"'); t.equal(obj.name, '', '.name should be empty String'); diff --git a/test/handlers/pkg.put.js b/test/handlers/pkg.put.js new file mode 100644 index 00000000..92c1fa81 --- /dev/null +++ b/test/handlers/pkg.put.js @@ -0,0 +1,152 @@ +'use strict'; + +const { PassThrough } = require('stream'); +const FormData = require('form-data'); +const HttpError = require('http-errors'); +const path = require('path'); +const tap = require('tap'); +const fs = require('fs'); + +const Handler = require('../../lib/handlers/pkg.put.js'); +const Sink = require('../../lib/sinks/test'); + +const FIXTURE_TAR = path.resolve(__dirname, '../../fixtures/package.tar'); +const FIXTURE_BZ2 = path.resolve(__dirname, '../../fixtures/package.tar.bz2'); +const FIXTURE_GZ = path.resolve(__dirname, '../../fixtures/package.tar.gz'); + + +const FIXTURE_PKG = path.resolve(__dirname, '../../fixtures/archive.tgz'); +const FIXTURE_MAP = path.resolve(__dirname, '../../fixtures/import-map.json'); + + +const Request = class Request extends PassThrough { + constructor ({ + headers = {} + } = {}) { + super(); + this.headers = {host: 'localhost', ...headers}; + } +} + + +tap.test('pkg.put() - The "type" argument is invalid', (t) => { + const h = new Handler(); + t.rejects(h.handler({}, 'anton', 'zaaap', 'fuzz', '8.4.1'), new HttpError.BadRequest(), 'should reject with bad request error'); + t.end(); +}); + +tap.test('pkg.put() - The "name" argument is invalid', (t) => { + const h = new Handler(); + t.rejects(h.handler({}, 'anton', 'pkg', null, '8.4.1'), new HttpError.BadRequest(), 'should reject with bad request error'); + t.end(); +}); + +tap.test('pkg.put() - The "version" argument is invalid', (t) => { + const h = new Handler(); + t.rejects(h.handler({}, 'anton', 'pkg', 'fuzz', 'zaaap'), new HttpError.BadRequest(), 'should reject with bad request error'); + t.end(); +}); + +tap.test('pkg.put() - Successful upload of .tar file', async (t) => { + const sink = new Sink(); + const h = new Handler({ sink }); + + const formData = new FormData(); + formData.append('package', fs.createReadStream(FIXTURE_TAR)); + + const headers = formData.getHeaders(); + const req = new Request({ headers }); + formData.pipe(req); + + const res = await h.handler(req, 'anton', 'pkg', 'fuzz', '8.4.1'); + + t.equal(res.cacheControl, 'no-store', '.cacheControl should be "no-store"') + t.equal(res.statusCode, 303, '.statusCode should be "303"'); + t.equal(res.mimeType, 'text/plain', '.mimeType should be "text/plain"'); + t.equal(res.location, '/pkg/fuzz/8.4.1', '.location should be "/pkg/fuzz/8.4.1"'); + t.end(); +}); + +tap.test('pkg.put() - Successful upload of .tar.gz file', async (t) => { + const sink = new Sink(); + const h = new Handler({ sink }); + + const formData = new FormData(); + formData.append('package', fs.createReadStream(FIXTURE_GZ)); + + const headers = formData.getHeaders(); + const req = new Request({ headers }); + formData.pipe(req); + + const res = await h.handler(req, 'anton', 'pkg', 'fuzz', '8.4.1'); + + t.equal(res.cacheControl, 'no-store', '.cacheControl should be "no-store"') + t.equal(res.statusCode, 303, '.statusCode should be "303"'); + t.equal(res.mimeType, 'text/plain', '.mimeType should be "text/plain"'); + t.equal(res.location, '/pkg/fuzz/8.4.1', '.location should be "/pkg/fuzz/8.4.1"'); + t.end(); +}); + +tap.test('pkg.put() - File is not a tar file', (t) => { + const sink = new Sink(); + const h = new Handler({ sink }); + + const formData = new FormData(); + formData.append('package', fs.createReadStream(FIXTURE_MAP)); + + const headers = formData.getHeaders(); + const req = new Request({ headers }); + formData.pipe(req); + + t.rejects(h.handler(req, 'anton', 'pkg', 'fuzz', '8.4.1'), new HttpError.UnsupportedMediaType(), 'should reject with unsupported media type error'); + t.end(); +}); + +tap.test('pkg.put() - File is not a compatible file or contain an error', (t) => { + const sink = new Sink(); + const h = new Handler({ sink }); + + const formData = new FormData(); + formData.append('package', fs.createReadStream(FIXTURE_BZ2)); + + const headers = formData.getHeaders(); + const req = new Request({ headers }); + formData.pipe(req); + + t.rejects(h.handler(req, 'anton', 'pkg', 'fuzz', '8.4.1'), new HttpError.UnprocessableEntity(), 'should reject with unprocessable entry error'); + t.end(); +}); + +tap.test('pkg.put() - Form field is not valid', (t) => { + const sink = new Sink(); + const h = new Handler({ sink }); + + const formData = new FormData(); + formData.append('pkg', fs.createReadStream(FIXTURE_PKG)); + + const headers = formData.getHeaders(); + const req = new Request({ headers }); + formData.pipe(req); + + t.rejects(h.handler(req, 'anton', 'pkg', 'fuzz', '8.4.1'), new HttpError.BadRequest(), 'should reject with bad request error'); + t.end(); +}); + +tap.test('pkg.put() - File exceeds legal file size limit', (t) => { + const sink = new Sink(); + const h = new Handler({ + pkgMaxFileSize: 100, + sink + }); + + const formData = new FormData(); + formData.append('package', fs.createReadStream(FIXTURE_PKG)); + + const headers = formData.getHeaders(); + const req = new Request({ headers }); + formData.pipe(req); + + t.rejects(h.handler(req, 'anton', 'pkg', 'fuzz', '8.4.1'), new HttpError.PayloadTooLarge(), 'should reject with payload too large error'); + t.end(); +}); + diff --git a/test/sinks/fs.js b/test/sinks/fs.js index 213cc494..1c59741e 100644 --- a/test/sinks/fs.js +++ b/test/sinks/fs.js @@ -71,6 +71,15 @@ test('Sink() - .write()', async (t) => { t.end(); }); +test('Sink() - .write() - arguments is illegal', async (t) => { + const sink = new Sink(DEFAULT_CONFIG); + const dir = slug(); + + t.rejects(sink.write(300, 'application/octet-stream'), new TypeError('Argument must be a String'), 'should reject on illegal filepath'); + t.rejects(sink.write(`${dir}/bar/map.json`, 300), new TypeError('Argument must be a String'), 'should reject on illegal mime type'); + t.end(); +}); + test('Sink() - .write() - directory traversal prevention', async (t) => { const sink = new Sink(DEFAULT_CONFIG); const dir = slug(); @@ -120,6 +129,34 @@ test('Sink() - .read() - File does NOT exist', (t) => { t.end(); }); +test('Sink() - .read() - arguments is illegal', async (t) => { + const sink = new Sink(DEFAULT_CONFIG); + t.rejects(sink.read(300), new TypeError('Argument must be a String'), 'should reject on illegal filepath'); + t.end(); +}); + +test('Sink() - .read() - directory traversal prevention', async (t) => { + const sink = new Sink(DEFAULT_CONFIG); + const dir = slug(); + const file = `${dir}/map.json`; + + const writeFrom = readFileStream('../../fixtures/import-map.json'); + const writeTo = await sink.write(file, 'application/json'); + + await pipe(writeFrom, writeTo); + + t.rejects(sink.read(`../../${dir}/sensitive.data`), new Error('Directory traversal'), 'should reject on ../../ at beginning of filepath'); + t.rejects(sink.read(`../${dir}/sensitive.data`), new Error('Directory traversal'), 'should reject on ../ at beginning of filepath'); + t.rejects(sink.read(`/${dir}/../../../foo/sensitive.data`), new Error('Directory traversal'), 'should reject on path traversal in the middle of filepath'); + t.resolves(sink.read(`./${file}`), 'should resolve on ./ at beginning of filepath'); + t.resolves(sink.read(`/${file}`), 'should resolve on / at beginning of filepath'); + t.resolves(sink.read(`//${file}`), 'should resolve on // at beginning of filepath'); + + // Clean up sink + await sink.delete(dir); + t.end(); +}); + test('Sink() - .read() - value of .mimeType of known file type', async (t) => { const sink = new Sink(DEFAULT_CONFIG); const dir = slug(); @@ -182,28 +219,6 @@ test('Sink() - .delete() - Delete existing file', async (t) => { t.end(); }); -test('Sink() - .read() - directory traversal prevention', async (t) => { - const sink = new Sink(DEFAULT_CONFIG); - const dir = slug(); - const file = `${dir}/map.json`; - - const writeFrom = readFileStream('../../fixtures/import-map.json'); - const writeTo = await sink.write(file, 'application/json'); - - await pipe(writeFrom, writeTo); - - t.rejects(sink.read(`../../${dir}/sensitive.data`), new Error('Directory traversal'), 'should reject on ../../ at beginning of filepath'); - t.rejects(sink.read(`../${dir}/sensitive.data`), new Error('Directory traversal'), 'should reject on ../ at beginning of filepath'); - t.rejects(sink.read(`/${dir}/../../../foo/sensitive.data`), new Error('Directory traversal'), 'should reject on path traversal in the middle of filepath'); - t.resolves(sink.read(`./${file}`), 'should resolve on ./ at beginning of filepath'); - t.resolves(sink.read(`/${file}`), 'should resolve on / at beginning of filepath'); - t.resolves(sink.read(`//${file}`), 'should resolve on // at beginning of filepath'); - - // Clean up sink - await sink.delete(dir); - t.end(); -}); - test('Sink() - .delete() - Delete non existing file', (t) => { const sink = new Sink(DEFAULT_CONFIG); t.resolves(sink.delete('/bar/foo/not-exist.json'), 'should resolve'); @@ -256,6 +271,12 @@ test('Sink() - .delete() - Delete files recursively', async (t) => { t.end(); }); +test('Sink() - .delete() - arguments is illegal', async (t) => { + const sink = new Sink(DEFAULT_CONFIG); + t.rejects(sink.delete(300), new TypeError('Argument must be a String'), 'should reject on illegal filepath'); + t.end(); +}); + test('Sink() - .delete() - directory traversal prevention', async (t) => { const sink = new Sink(DEFAULT_CONFIG); const dir = slug(); @@ -296,6 +317,12 @@ test('Sink() - .exist() - Check non existing file', (t) => { t.end(); }); +test('Sink() - .exist() - arguments is illegal', async (t) => { + const sink = new Sink(DEFAULT_CONFIG); + t.rejects(sink.exist(300), new TypeError('Argument must be a String'), 'should reject on illegal filepath'); + t.end(); +}); + test('Sink() - .exist() - directory traversal prevention', async (t) => { const sink = new Sink(DEFAULT_CONFIG); const dir = slug(); diff --git a/test/sinks/mem.js b/test/sinks/mem.js index 737dbb36..6aa328f8 100644 --- a/test/sinks/mem.js +++ b/test/sinks/mem.js @@ -68,6 +68,15 @@ test('Sink() - .write()', async (t) => { t.end(); }); +test('Sink() - .write() - arguments is illegal', async (t) => { + const sink = new Sink(DEFAULT_CONFIG); + const dir = slug(); + + t.rejects(sink.write(300, 'application/octet-stream'), new TypeError('Argument must be a String'), 'should reject on illegal filepath'); + t.rejects(sink.write(`${dir}/bar/map.json`, 300), new TypeError('Argument must be a String'), 'should reject on illegal mime type'); + t.end(); +}); + test('Sink() - .write() - directory traversal prevention', async (t) => { const sink = new Sink(DEFAULT_CONFIG); const dir = slug(); @@ -117,64 +126,70 @@ test('Sink() - .read() - File does NOT exist', (t) => { t.end(); }); -test('Sink() - .read() - value of .mimeType of known file type', async (t) => { +test('Sink() - .read() - arguments is illegal', async (t) => { const sink = new Sink(DEFAULT_CONFIG); - const dir = slug(); - const file = `${dir}/bar/map.json`; + t.rejects(sink.read(300), new TypeError('Argument must be a String'), 'should reject on illegal filepath'); + t.end(); +}); +test('Sink() - .read() - directory traversal prevention', async (t) => { + const sink = new Sink(DEFAULT_CONFIG); + const dir = slug(); + const file = `${dir}/map.json`; const writeFrom = readFileStream('../../fixtures/import-map.json'); const writeTo = await sink.write(file, 'application/json'); await pipe(writeFrom, writeTo); - const readFrom = await sink.read(file); - - t.equal(readFrom.mimeType, 'application/json', 'should resolve with a ReadFile object which has a value for .mimeType matching the file'); + t.rejects(sink.read(`../../${dir}/sensitive.data`), new Error('Directory traversal'), 'should reject on ../../ at beginning of filepath'); + t.rejects(sink.read(`../${dir}/sensitive.data`), new Error('Directory traversal'), 'should reject on ../ at beginning of filepath'); + t.rejects(sink.read(`/${dir}/../../../foo/sensitive.data`), new Error('Directory traversal'), 'should reject on path traversal in the middle of filepath'); + t.resolves(sink.read(`./${file}`), 'should resolve on ./ at beginning of filepath'); + t.resolves(sink.read(`/${file}`), 'should resolve on / at beginning of filepath'); + t.resolves(sink.read(`//${file}`), 'should resolve on // at beginning of filepath'); // Clean up sink await sink.delete(dir); t.end(); }); -test('Sink() - .delete() - Delete existing file', async (t) => { +test('Sink() - .read() - value of .mimeType of known file type', async (t) => { const sink = new Sink(DEFAULT_CONFIG); - const dir = slug(); const file = `${dir}/bar/map.json`; + const writeFrom = readFileStream('../../fixtures/import-map.json'); const writeTo = await sink.write(file, 'application/json'); await pipe(writeFrom, writeTo); - t.resolves(sink.exist(file), 'should resolve - file is in sink before deletion'); - - await sink.delete(file); + const readFrom = await sink.read(file); - t.rejects(sink.exist(file), 'should reject - file was deleted'); + t.equal(readFrom.mimeType, 'application/json', 'should resolve with a ReadFile object which has a value for .mimeType matching the file'); // Clean up sink await sink.delete(dir); t.end(); }); -test('Sink() - .read() - directory traversal prevention', async (t) => { +test('Sink() - .delete() - Delete existing file', async (t) => { const sink = new Sink(DEFAULT_CONFIG); + const dir = slug(); - const file = `${dir}/map.json`; + const file = `${dir}/bar/map.json`; const writeFrom = readFileStream('../../fixtures/import-map.json'); const writeTo = await sink.write(file, 'application/json'); await pipe(writeFrom, writeTo); - t.rejects(sink.read(`../../${dir}/sensitive.data`), new Error('Directory traversal'), 'should reject on ../../ at beginning of filepath'); - t.rejects(sink.read(`../${dir}/sensitive.data`), new Error('Directory traversal'), 'should reject on ../ at beginning of filepath'); - t.rejects(sink.read(`/${dir}/../../../foo/sensitive.data`), new Error('Directory traversal'), 'should reject on path traversal in the middle of filepath'); - t.resolves(sink.read(`./${file}`), 'should resolve on ./ at beginning of filepath'); - t.resolves(sink.read(`/${file}`), 'should resolve on / at beginning of filepath'); - t.resolves(sink.read(`//${file}`), 'should resolve on // at beginning of filepath'); + t.resolves(sink.exist(file), 'should resolve - file is in sink before deletion'); + + await sink.delete(file); + + t.rejects(sink.exist(file), 'should reject - file was deleted'); // Clean up sink await sink.delete(dir); @@ -233,6 +248,12 @@ test('Sink() - .delete() - Delete files recursively', async (t) => { t.end(); }); +test('Sink() - .delete() - arguments is illegal', async (t) => { + const sink = new Sink(DEFAULT_CONFIG); + t.rejects(sink.delete(300), new TypeError('Argument must be a String'), 'should reject on illegal filepath'); + t.end(); +}); + test('Sink() - .delete() - directory traversal prevention', async (t) => { const sink = new Sink(DEFAULT_CONFIG); const dir = slug(); @@ -273,6 +294,12 @@ test('Sink() - .exist() - Check non existing file', (t) => { t.end(); }); +test('Sink() - .exist() - arguments is illegal', async (t) => { + const sink = new Sink(DEFAULT_CONFIG); + t.rejects(sink.exist(300), new TypeError('Argument must be a String'), 'should reject on illegal filepath'); + t.end(); +}); + test('Sink() - .exist() - directory traversal prevention', async (t) => { const sink = new Sink(DEFAULT_CONFIG); const dir = slug(); diff --git a/test/sinks/test.js b/test/sinks/test.js index 29dad677..e5c7cf6f 100644 --- a/test/sinks/test.js +++ b/test/sinks/test.js @@ -298,6 +298,15 @@ test('Sink() - .write()', async (t) => { t.end(); }); +test('Sink() - .write() - arguments is illegal', async (t) => { + const sink = new Sink(DEFAULT_CONFIG); + const dir = slug(); + + t.rejects(sink.write(300, 'application/octet-stream'), new TypeError('Argument must be a String'), 'should reject on illegal filepath'); + t.rejects(sink.write(`${dir}/bar/map.json`, 300), new TypeError('Argument must be a String'), 'should reject on illegal mime type'); + t.end(); +}); + test('Sink() - .write() - directory traversal prevention', async (t) => { const sink = new Sink(DEFAULT_CONFIG); const dir = slug(); @@ -347,64 +356,70 @@ test('Sink() - .read() - File does NOT exist', (t) => { t.end(); }); -test('Sink() - .read() - value of .mimeType of known file type', async (t) => { +test('Sink() - .read() - arguments is illegal', async (t) => { const sink = new Sink(DEFAULT_CONFIG); - const dir = slug(); - const file = `${dir}/bar/map.json`; + t.rejects(sink.read(300), new TypeError('Argument must be a String'), 'should reject on illegal filepath'); + t.end(); +}); +test('Sink() - .read() - directory traversal prevention', async (t) => { + const sink = new Sink(DEFAULT_CONFIG); + const dir = slug(); + const file = `${dir}/map.json`; const writeFrom = readFileStream('../../fixtures/import-map.json'); const writeTo = await sink.write(file, 'application/json'); await pipe(writeFrom, writeTo); - const readFrom = await sink.read(file); - - t.equal(readFrom.mimeType, 'application/json', 'should resolve with a ReadFile object which has a value for .mimeType matching the file'); + t.rejects(sink.read(`../../${dir}/sensitive.data`), new Error('Directory traversal'), 'should reject on ../../ at beginning of filepath'); + t.rejects(sink.read(`../${dir}/sensitive.data`), new Error('Directory traversal'), 'should reject on ../ at beginning of filepath'); + t.rejects(sink.read(`/${dir}/../../../foo/sensitive.data`), new Error('Directory traversal'), 'should reject on path traversal in the middle of filepath'); + t.resolves(sink.read(`./${file}`), 'should resolve on ./ at beginning of filepath'); + t.resolves(sink.read(`/${file}`), 'should resolve on / at beginning of filepath'); + t.resolves(sink.read(`//${file}`), 'should resolve on // at beginning of filepath'); // Clean up sink await sink.delete(dir); t.end(); }); -test('Sink() - .delete() - Delete existing file', async (t) => { +test('Sink() - .read() - value of .mimeType of known file type', async (t) => { const sink = new Sink(DEFAULT_CONFIG); - const dir = slug(); const file = `${dir}/bar/map.json`; + const writeFrom = readFileStream('../../fixtures/import-map.json'); const writeTo = await sink.write(file, 'application/json'); await pipe(writeFrom, writeTo); - t.resolves(sink.exist(file), 'should resolve - file is in sink before deletion'); - - await sink.delete(file); + const readFrom = await sink.read(file); - t.rejects(sink.exist(file), 'should reject - file was deleted'); + t.equal(readFrom.mimeType, 'application/json', 'should resolve with a ReadFile object which has a value for .mimeType matching the file'); // Clean up sink await sink.delete(dir); t.end(); }); -test('Sink() - .read() - directory traversal prevention', async (t) => { +test('Sink() - .delete() - Delete existing file', async (t) => { const sink = new Sink(DEFAULT_CONFIG); + const dir = slug(); - const file = `${dir}/map.json`; + const file = `${dir}/bar/map.json`; const writeFrom = readFileStream('../../fixtures/import-map.json'); const writeTo = await sink.write(file, 'application/json'); await pipe(writeFrom, writeTo); - t.rejects(sink.read(`../../${dir}/sensitive.data`), new Error('Directory traversal'), 'should reject on ../../ at beginning of filepath'); - t.rejects(sink.read(`../${dir}/sensitive.data`), new Error('Directory traversal'), 'should reject on ../ at beginning of filepath'); - t.rejects(sink.read(`/${dir}/../../../foo/sensitive.data`), new Error('Directory traversal'), 'should reject on path traversal in the middle of filepath'); - t.resolves(sink.read(`./${file}`), 'should resolve on ./ at beginning of filepath'); - t.resolves(sink.read(`/${file}`), 'should resolve on / at beginning of filepath'); - t.resolves(sink.read(`//${file}`), 'should resolve on // at beginning of filepath'); + t.resolves(sink.exist(file), 'should resolve - file is in sink before deletion'); + + await sink.delete(file); + + t.rejects(sink.exist(file), 'should reject - file was deleted'); // Clean up sink await sink.delete(dir); @@ -463,6 +478,12 @@ test('Sink() - .delete() - Delete files recursively', async (t) => { t.end(); }); +test('Sink() - .delete() - arguments is illegal', async (t) => { + const sink = new Sink(DEFAULT_CONFIG); + t.rejects(sink.delete(300), new TypeError('Argument must be a String'), 'should reject on illegal filepath'); + t.end(); +}); + test('Sink() - .delete() - directory traversal prevention', async (t) => { const sink = new Sink(DEFAULT_CONFIG); const dir = slug(); @@ -503,6 +524,12 @@ test('Sink() - .exist() - Check non existing file', (t) => { t.end(); }); +test('Sink() - .exist() - arguments is illegal', async (t) => { + const sink = new Sink(DEFAULT_CONFIG); + t.rejects(sink.exist(300), new TypeError('Argument must be a String'), 'should reject on illegal filepath'); + t.end(); +}); + test('Sink() - .exist() - directory traversal prevention', async (t) => { const sink = new Sink(DEFAULT_CONFIG); const dir = slug();