-
Notifications
You must be signed in to change notification settings - Fork 573
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
Listen on unix socket #325
Conversation
Perhaps the app should also stop using tcp by default. |
Codecov Report
@@ Coverage Diff @@
## master #325 +/- ##
==========================================
- Coverage 68.77% 68.23% -0.55%
==========================================
Files 16 16
Lines 823 831 +8
Branches 96 89 -7
==========================================
+ Hits 566 567 +1
- Misses 229 234 +5
- Partials 28 30 +2
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #325 +/- ##
==========================================
- Coverage 68.77% 68.47% -0.31%
==========================================
Files 16 16
Lines 823 831 +8
Branches 96 98 +2
==========================================
+ Hits 566 569 +3
- Misses 229 232 +3
- Partials 28 30 +2
Continue to review full report at Codecov.
|
integrated with current master here https://github.com/scrapy/scrapyd/compare/uds?expand=1 |
webservice = UNIXServer(uds_path, website, mode=0o660) | ||
log.msg(format=u"Scrapyd web console available at http+unix://%(uds_path)s", | ||
uds_path=uds_path) | ||
webservice.setServiceParent(app) |
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.
I'm not certain if I understand the logic here, is this really meant to start both TCPServer and UnixServer? Or is there some intention to allow to disable and stop TCPServer by setting http_port to zero? If 2nd option, if we want to add option to disable TCP this should be explicitly documented in docs.
Because with this implementation if someone just adds uds_path without removing http_port it will start both of them, unixserver and tcp. Is that expected?
The problem (authentication) seems to be equally solved by HTTP basic authentication, which is already merged. (The Unix socket strategy is to use filesystem permissions to prevent other users from connecting to the socket.) Since this is not related to any feature request issue (i.e. no clear demand), I'll close. |
Done in #510 |
This allows securing the app from users on the same system,
provided that the effective group id of scrapyd's process is properly chosen.