-
Notifications
You must be signed in to change notification settings - Fork 1
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
Restrinct net and write permissions in conract-related commands #10
Conversation
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.
Nice work @snowteamer. Merge conflicts + 1 minor issue
src/upload.ts
Outdated
import { blake32Hash, isDir, revokeNet } from './utils.ts' | ||
|
||
// chel upload <url-or-dir-or-sqlitedb> <file1> [<file2> [<file3> ...]] | ||
|
||
// TODO: use Deno.permissions.request(...) to request permissions to the specific URL | ||
// https://deno.land/manual/runtime/permission_apis | ||
// and use this everywhere so that we protect against malicious contracts | ||
|
||
export async function upload (args: string[], internal = false) { | ||
await revokeNet() |
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.
Uploading makes sense to have net permissions though. This function, revokeNet
, is mainly useful for when we implement latestState
I think
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.
Hmm. I did write
See also comment in
src/upload.ts
And I did write that comment. No idea why I wrote that in this file though 😅
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.
Calling revokeNet()
here ensures that the user gets a prompt so that only the upload URL gets allowed. Otherwise, no prompt would show up, but any URL would be implicitly allowed.
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, but if we are using chel upload
we expect a connection to be made. Might be annoying to have to confirm. Not only annoying, but if this is used as part of a build step, would be disruptive
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.
LGTM! Great job @snowteamer!
Closes #1