-
Notifications
You must be signed in to change notification settings - Fork 325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: scope mfs calls to an application path #431
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
0fd8f6c
adds function to scope mfs calls to an application path
alanshaw bb82619
updates ipfs-postmsg-proxy
alanshaw e079d9c
adds MFS scope docs
alanshaw 656b891
changes wording and adds link
alanshaw 3dd7ecc
adds mfs scope tests
alanshaw 36eb609
adds default case and comments
alanshaw 53d3326
ensure app path exists, URI encode path segs for safe directory names
alanshaw ff7ed6f
tweaks mfs path to remove : from protocol
alanshaw 07f0d26
removes .only
alanshaw File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,103 @@ | ||
const Path = require('path') | ||
const DEFAULT_ROOT_PATH = '/dapps' | ||
|
||
// Creates a "pre" function that is called prior to calling a real function | ||
// on the IPFS instance. It modifies the arguments to MFS functions to scope | ||
// file access to a directory designated to the web page | ||
function createPreMfsScope (fnName, getScope, getIpfs, rootPath = DEFAULT_ROOT_PATH) { | ||
return MfsPre[fnName] ? MfsPre[fnName](getScope, getIpfs, rootPath) : null | ||
} | ||
|
||
module.exports = createPreMfsScope | ||
|
||
const MfsPre = { | ||
'files.cp': createSrcDestPre, | ||
'files.mkdir': createSrcPre, | ||
'files.stat': createSrcPre, | ||
'files.rm' (getScope, getIpfs, rootPath) { | ||
const srcPre = createSrcPre(getScope, getIpfs, rootPath) | ||
// Do not allow rm app root | ||
// Need to explicitly deny because it's ok to rm -rf /a/path that's not / | ||
return (...args) => { | ||
if (isRoot(args[0])) throw new Error('cannot delete root') | ||
return srcPre(...args) | ||
} | ||
}, | ||
'files.read': createSrcPre, | ||
'files.write' (getScope, getIpfs, rootPath) { | ||
const srcPre = createSrcPre(getScope, getIpfs, rootPath) | ||
// Do not allow write to app root | ||
// Need to explicitly deny because app path might not exist yet | ||
return (...args) => { | ||
if (isRoot(args[0])) throw new Error('/ was not a file') | ||
return srcPre(...args) | ||
} | ||
}, | ||
'files.mv': createSrcDestPre, | ||
'files.flush': createOptionalSrcPre, | ||
'files.ls': createOptionalSrcPre | ||
} | ||
|
||
// Scope a src/dest tuple to the app path | ||
function createSrcDestPre (getScope, getIpfs, rootPath) { | ||
return async (...args) => { | ||
const appPath = await getAppPath(getScope, getIpfs, rootPath) | ||
args[0][0] = Path.join(appPath, safePath(args[0][0])) | ||
args[0][1] = Path.join(appPath, safePath(args[0][1])) | ||
return args | ||
} | ||
} | ||
|
||
// Scope a src path to the app path | ||
function createSrcPre (getScope, getIpfs, rootPath) { | ||
return async (...args) => { | ||
const appPath = await getAppPath(getScope, getIpfs, rootPath) | ||
args[0] = Path.join(appPath, safePath(args[0])) | ||
return args | ||
} | ||
} | ||
|
||
// Scope an optional src path to the app path | ||
function createOptionalSrcPre (getScope, getIpfs, rootPath) { | ||
return async (...args) => { | ||
const appPath = await getAppPath(getScope, getIpfs, rootPath) | ||
|
||
if (Object.prototype.toString.call(args[0]) === '[object String]') { | ||
args[0] = Path.join(appPath, safePath(args[0])) | ||
} else { | ||
switch (args.length) { | ||
case 0: return [appPath] // e.g. ipfs.files.ls() | ||
case 1: return [appPath, args[0]] // e.g. ipfs.files.ls(options) | ||
case 2: return [appPath, args[1]] // e.g. ipfs.files.ls(null, options) | ||
default: throw new Error('Unexpected number of arguments') | ||
} | ||
} | ||
return args | ||
} | ||
} | ||
|
||
// Get the app path (create if not exists) for a scope, prefixed with rootPath | ||
const getAppPath = async (getScope, getIpfs, rootPath) => { | ||
const appPath = rootPath + scopeToPath(await getScope()) | ||
await getIpfs().files.mkdir(appPath, { parents: true }) | ||
return appPath | ||
} | ||
|
||
// Turn http://ipfs.io/ipfs/QmUmaEnH1uMmvckMZbh3yShaasvELPW4ZLPWnB4entMTEn | ||
// into /http/ipfs.io/ipfs/QmUmaEnH1uMmvckMZbh3yShaasvELPW4ZLPWnB4entMTEn | ||
const scopeToPath = (scope) => { | ||
return ('/' + scope) | ||
.replace(/\/\//g, '/') | ||
.split('/') | ||
// Special case for protocol in scope, remove : from the end | ||
.map((seg, i) => i === 1 && seg.endsWith(':') ? seg.slice(0, -1) : seg) | ||
.map(encodeURIComponent) | ||
.join('/') | ||
} | ||
|
||
// Make a path "safe" by resolving any directory traversal segments relative to | ||
// '/'. Allows us to then prefix the app path without worrying about the user | ||
// breaking out of their jail. | ||
const safePath = (path) => Path.resolve('/', path) | ||
|
||
const isRoot = (path) => Path.resolve('/', path) === '/' |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question; how will it behave when a new API
ipfs.files.foo
is added ?Will it not work, or will it work with no path scoping?
If the latter is the case, we should explicitly block executions of new methods in scoped
ipfs.files
API to avoid surprises in future.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I mean, you'd need to update the
ipfs-postmsg-proxy
dependency to allow the new method to be used in the first place, but no it wouldn't be scoped automatically unless you did some work here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, being dependent on
ipfs-postmsg-proxy
makes this safe enough for now.Unless its trivial to add, I propose we add additional safeguards in separate PR.