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.

Sign up for GitHub

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

Jump to bottom

feat(doctor) content folder permission checks #610

Merged
merged 1 commit into from
Feb 4, 2018
Merged

feat(doctor) content folder permission checks #610

merged 1 commit into from
Feb 4, 2018

Conversation

aileen
Copy link
Member

@aileen aileen commented Feb 2, 2018

refs #47

  • adds a new task to ghost doctor startup which runs on ghost start
  • skips this task if
    • ghost user is not setup or
    • ghost user doesn't own the content folder
  • checks the permissions inside the content folder and fails if a file or directory is not owned by ghost
  • logs the relevant files or directories
  • adds tests

@aileen aileen requested a review from acburdine February 2, 2018 17:20
@coveralls
Copy link

coveralls commented Feb 2, 2018 •
edited
Loading

Coverage Status

Coverage increased (+0.02%) to 98.681% when pulling f55da27 on AileenCGN:content-folder-permissions into 76829e9 on TryGhost:master.

acburdine
acburdine approved these changes Feb 2, 2018
View reviewed changes
Copy link
Member

@acburdine acburdine left a comment

Choose a reason for hiding this comment

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

Code looks good - will test in the morning 👍

@acburdine acburdine mentioned this pull request Feb 3, 2018
Closed
18 tasks
aileen added a commit to aileen/Ghost-CLI that referenced this pull request Feb 3, 2018
@aileen
feat(doctor) detect incorrect permissions
a3eb47b 
refs TryGhost#47
refs TryGhost#294
needs TryGhost#610

- Refactors the `contentFolderPermissions` to be named `checkPermissions` which takes a property to specify which check should run:
	- `owner` checks for ownership of the content folder (should be owned by the `ghost` user and group, if `ghost` user is set up. See TryGhost#610)
	- `folder` checks if directories within the ghost installation folder have the correct permission (775)
	- `files` checks if files within the ghost installation folder have the correct permissions (664)
- Adds tests
aileen added a commit to aileen/Ghost-CLI that referenced this pull request Feb 3, 2018
@aileen
feat(doctor) detect incorrect permissions
edf8884 
refs TryGhost#47
refs TryGhost#294
needs TryGhost#610

- Refactors the `contentFolderPermissions` to be named `checkPermissions` which takes a property to specify which check should run:
	- `owner` checks for ownership of the content folder (should be owned by the `ghost` user and group, if `ghost` user is set up. See TryGhost#610)
	- `folder` checks if directories within the ghost installation folder have the correct permission (775)
	- `files` checks if files within the ghost installation folder have the correct permissions (664)
- Adds tests
aileen added a commit to aileen/Ghost-CLI that referenced this pull request Feb 3, 2018
@aileen
feat(doctor) detect incorrect permissions
aa80666 
refs TryGhost#47
refs TryGhost#294
needs TryGhost#610

- Refactors the `contentFolderPermissions` to be named `checkPermissions` which takes a property to specify which check should run:
	- `owner` checks for ownership of the content folder (should be owned by the `ghost` user and group, if `ghost` user is set up. See TryGhost#610)
	- `folder` checks if directories within the ghost installation folder have the correct permission (775)
	- `files` checks if files within the ghost installation folder have the correct permissions (664)
- Adds tests
aileen added a commit to aileen/Ghost-CLI that referenced this pull request Feb 3, 2018
@aileen
feat(doctor) detect incorrect permissions
0a3173c 
refs TryGhost#47
refs TryGhost#294
needs TryGhost#610

- Refactors the `contentFolderPermissions` to be named `checkPermissions` which takes a property to specify which check should run:
	- `owner` checks for ownership of the content folder (should be owned by the `ghost` user and group, if `ghost` user is set up. See TryGhost#610)
	- `folder` checks if directories within the ghost installation folder have the correct permission (775)
	- `files` checks if files within the ghost installation folder have the correct permissions (664)
- Adds tests
@aileen aileen mentioned this pull request Feb 3, 2018
Merged
aileen added a commit to aileen/Ghost-CLI that referenced this pull request Feb 4, 2018
@aileen
feat(doctor) detect incorrect permissions
fbad910 
refs TryGhost#47
refs TryGhost#294
needs TryGhost#610

- Refactors the `contentFolderPermissions` to be named `checkPermissions` which takes a property to specify which check should run:
	- `owner` checks for ownership of the content folder (should be owned by the `ghost` user and group, if `ghost` user is set up. See TryGhost#610)
	- `folder` checks if directories within the ghost installation folder have the correct permission (775)
	- `files` checks if files within the ghost installation folder have the correct permissions (664)
- Adds tests
@aileen aileen force-pushed the content-folder-permissions branch 4 times, most recently from e0a7523 to 1763f48 Compare February 4, 2018 11:22
acburdine
acburdine reviewed Feb 4, 2018
View reviewed changes
lib/commands/doctor/checks/content-folder.js Outdated

module.exports = {
title: 'Content folder permissions',
enabled: () => {

This comment was marked as abuse.

Sign in to view

@aileen
feat(doctor) content folder permission checks
f55da27 
refs TryGhost#47

- adds a new task to `ghost doctor startup` which runs on `ghost start`
- skips this task if
	- ghost user is not setup or
	- ghost user doesn't own the content folder
- checks the permissions inside the content folder and fails if a file or directory is not owned by ghost
- logs the relevant files or directories
- adds tests
acburdine
acburdine approved these changes Feb 4, 2018
View reviewed changes
Copy link
Member

@acburdine acburdine left a comment

Choose a reason for hiding this comment

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

👍

@acburdine acburdine merged commit 0075e77 into TryGhost:master Feb 4, 2018
aileen added a commit to aileen/Ghost-CLI that referenced this pull request Feb 4, 2018
@aileen
feat(doctor) detect incorrect permissions
2ede701 
refs TryGhost#47
refs TryGhost#294
needs TryGhost#610

- Refactors the `contentFolderPermissions` to be named `checkPermissions` which takes a property to specify which check should run:
	- `owner` checks for ownership of the content folder (should be owned by the `ghost` user and group, if `ghost` user is set up. See TryGhost#610)
	- `folder` checks if directories within the ghost installation folder have the correct permission (775)
	- `files` checks if files within the ghost installation folder have the correct permissions (664)
- Adds tests
aileen added a commit to aileen/Ghost-CLI that referenced this pull request Feb 4, 2018
@aileen
feat(doctor) detect incorrect permissions
8f980c1 
refs TryGhost#47
refs TryGhost#294
needs TryGhost#610

- Refactors the `contentFolderPermissions` to be named `checkPermissions` which takes a property to specify which check should run:
	- `owner` checks for ownership of the content folder (should be owned by the `ghost` user and group, if `ghost` user is set up. See TryGhost#610)
	- `folder` checks if directories within the ghost installation folder have the correct permission (775)
	- `files` checks if files within the ghost installation folder have the correct permissions (664)
- Adds tests
@aileen aileen deleted the content-folder-permissions branch May 10, 2018 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acburdine acburdine acburdine approved these changes

Assignees

@acburdine acburdine

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aileen @coveralls @acburdine