We should just run through this and make sure instances can't adjust path settings for this stuff, and that it's all forced to some reasonable, namespaced path.
Description
Revisions and Commits
| Restricted Diffusion Commit | ||
| Restricted Differential Revision | Restricted Diffusion Commit | |
| Restricted Differential Revision | Restricted Diffusion Commit |
Event Timeline
It would probably be good if we both did a pass through this and compared notes. Basically:
- Look through all the config options.
- For options which are not "Locked", can any be set to something which would cause problems with instances? An example is if two instances can set a log file to the same path.
- We set some options in PhacilitySiteSource in rSERVICES, but currently do not add any locks. Most of the options we set should have locks added if they aren't already locked.
- Broadly, for each option, can you come up with a way that users could break their installs or cause trouble by adjusting it?
Once we have a list of things that should be locked, we can trivially lock it in PhacilitySiteSource.
This table are issues where I worried about different instances having the same value...
| key | concern |
| asana.project-ids | I think okay but worth raising |
| asana.workspace-id | I think okay but worth raising |
| metamta.default-address | could break per instance reply to email functionality |
| metamta.differential.reply-handler-domain | could break per instance reply to email functionality |
| metamta.diffusion.reply-handler-domain | could break per instance reply to email functionality |
| metamta.macro.reply-handler-domain | could break per instance reply to email functionality |
| metamta.maniphest.reply-handler-domain | could break per instance reply to email functionality |
| metamta.pholio.reply-handler-domain | could break per instance reply to email functionality |
| metamta.reply-handler-domain | could break per instance reply to email functionality |
| notification.log | log collision |
| notification.pidfile | pid collision |
| phd.log-directory | log collision |
| phd.pid-directory | pid collision |
| sms.default-sender | could break per instance reply to SMS functionality |
| storage.s3.bucket | should be unique per instance probably |
This table is config that the users can basically break their instance for fun in some way right now?
| key | concern |
| metamta.differential.reply-handler | can't really add alternate handler |
| metamta.diffusion.reply-handler | can't really add alternate handler |
| metamta.maniphest.reply-handler | can't really add alternate handler |
| metamta.package.reply-handler | can't really add alternate handler |
Here's my list. We should probably lock these:
asana.project-ids
asana.workspace-id
celerity.minify
celerity.resource-hash
metamta.mail-adapter
phd.variant-config
phd.trace
phd.verbose
search.engine-selector
sms.default-adapter
sms.default-sender
storage.mysql-engine.max-size
syntax-highlighter.engine
...and probably adjust and lock these:
diffusion.ssh-port
diffusion.ssh-user
files.enable-imagemagick
log.access.path
log.ssh.path
metamta.default-address
metamta.domain
metamta.single-reply-handler-prefix
notification.client-uri
notification.log
notification.pidfile
notification.server-uri
notification.ssl-cert
notification.ssl-key
phabricator.production-uri
phd.log-directory
phd.pid-directory
phd.start-taskmasters
storage.engine-selector
storage.s3.bucket
storage.upload-size-limit
...and adjust (but not lock) this:
notification.enabled
...and maybe lock these some day although they seem fine for now:
debug.profile-rate
debug.stop-on-redirect
debug.time-limit
...and probably lock these in the upstream:
*.reply-handler (on the way out, not very useful anyway)
*.reply-handler-domain (could be used by a compromised administrator account to trick users to replying to mail and sending it to an attacker)
phabricator.cookie-prefix (can kind-of-break installs)
Lists look pretty similar, I was just a little more aggressive. A couple things are blocked on S3/Aphlict. I agree on Asana being not-dangerous but think it's worth locking to avoid confusion.