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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize to standar file permissions #1987

Merged
merged 2 commits into from Sep 24, 2018
Merged

Normalize to standar file permissions #1987

merged 2 commits into from Sep 24, 2018

Conversation

maturanomx
Copy link
Contributor

As a result of taking a look on PR #1967 I realized there is 819
executable files in this repository. It is obvious this is an error.

My PR is a:

馃拝 Enhancement

Main update on the:

Framework

@lauriejim
Copy link
Contributor

I'm not sur to 馃挴% understand your PR and the update you did.
Can you details please. The file change is not understandable.

@lauriejim lauriejim self-assigned this Sep 21, 2018
@lauriejim lauriejim self-requested a review September 21, 2018 15:44
@lauriejim lauriejim added pr: 馃拝 Enhancement source: core:strapi Source is core/strapi package labels Sep 21, 2018
@maturanomx
Copy link
Contributor Author

maturanomx commented Sep 21, 2018

馃 I not sure how to answer that... I will try explain it, sorry if I'm misunderstood the comment.

  1. Unix systems have a file permissions system. There are 3 basic types: write, read and execution; and 3 type of access: owner, group and others.

  2. Right now, with a clean clone from master all the listed files are executable. Let's take the file LICENSE.md in the directory root as example:
    capture

The actual permissions for that file -rwxr-xr-x are (or 775), meaning:

  • Can been write (edited/deleted) by the owner (maturano) and users in the group maturano
  • Can been read just by the owner and nobody else
  • Can been execute by anybody

Mainly, two problems with the last one:

  • It's not an executable file. Is not code, is just plain text.
  • If were executable, anyone could run it
    capture
    (Error syntax because obviously is not valid code, but you can see how is trying to running it)

You can see in the image the user postgres is also able to execute the file. In a shared system (everyday less common, I know) this is a security risk. I took an invalid file, but if were a real executable...

This PR just change fix the file permissions. You can see the change in the side of the filename:
capture

From 775 (or -rwxr-xr-x) to 664 (or -rw-r--r--), meaning:

  • Anybody can read it
  • Just the owner of the file can write it
  • Nobody can execute it...
    capture

Hope this clarify the PR and why I created it. This is --basic?-- Unix and could be an open door for insecurity issues. These days with devops, virtualization, container, clouds and bla, bla, bla is less important dangerous, but doing our part in our side doesn't hurt 馃槈


... and yes, Windows doesn't have this file attributes and if you write/move a file from Windows to Unix it will put it as an executable and ... 馃槳

As a result of taking a look on PR #1967 I realized there is 819
executable files in this repository. It is obvious this is an error.
@lauriejim
Copy link
Contributor

Really clear! Thank you @maturanomx

Copy link
Contributor

@lauriejim lauriejim left a comment

Choose a reason for hiding this comment

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

That is good to me @maturanomx , thank you for this PR.

@lauriejim lauriejim added this to In progress in 3.0.0-alpha.14.2 via automation Sep 24, 2018
@lauriejim lauriejim added this to the 3.0.0-alpha.14.2 milestone Sep 24, 2018
@lauriejim lauriejim merged commit 124d0b1 into strapi:master Sep 24, 2018
3.0.0-alpha.14.2 automation moved this from In progress to Done Sep 24, 2018
@maturanomx maturanomx deleted the fixFilePermissions branch September 24, 2018 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
source: core:strapi Source is core/strapi package
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants