Skip to content
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

Secure whitelist skeleton templates #584

Merged
merged 7 commits into from
Nov 7, 2016
Merged

Conversation

kpalway
Copy link
Contributor

@kpalway kpalway commented Nov 4, 2016

Uses a system call to SCP to copy the files from the course account instead of downloading them from a publicly readable URL.

Copy link
Contributor

@e45lee e45lee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably should be able to do something cleaner using curl and the sftp://... URL scheme that curl supports -- with that, you probably don't need to modify project.rkt and can just modify templates.rkt. If there's time, I'd prefer if you did that. If not, I'm OK with merging this now and coming back to this later.

(define/contract (scp-whitelist-project name source)
(-> project-name? remote-path-string? path-string?)
(define zip-path (path->string (make-temporary-file (string-append "rkttmp~a-" name ".zip"))))
(if (system* (find-executable-path "scp")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put the path in seashell-config.rkt.in?

;; Returns the path of the file copied locally
(define/contract (scp-whitelist-project name source)
(-> project-name? remote-path-string? path-string?)
(define zip-path (path->string (make-temporary-file (string-append "rkttmp~a-" name ".zip"))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe instead of creating a temporary file pipe it into a racket port? Something like ssh cs136@... cat <path> or curl sftp://cs136@.../path/to/file?

@kpalway
Copy link
Contributor Author

kpalway commented Nov 6, 2016

I should have time to change it to use curl

@kpalway
Copy link
Contributor Author

kpalway commented Nov 6, 2016

Looks like curl doesn't support sftp:// or scp:// out of the box, so the binary on the linux.student.cs environment won't work with it. Instead I used ssh to clean things up.

I still need to test the behaviour of this when authentication fails, so wait to merge.

(define-values (sshproc sshout sshin ssherr)
(subprocess #f #f #f (read-config 'ssh-binary)
host (string-append "cat " file)))
(thunk sshout)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure you close sshout/ssherr and you use dynamic-wind (see below) to close sshout when thunk returns.

@e45lee e45lee merged commit 9d24a2f into cs136:master Nov 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants