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

Fixed invalid CSS, bumped stylus #712

Closed
wants to merge 2 commits into from
Closed

Conversation

nyurik
Copy link

@nyurik nyurik commented Sep 15, 2015

"highlight-color += 10%" in context-menu.styl produces
an invalid color entry in CSS

Yuri Astrakhan added 2 commits September 15, 2015 08:32
"highlight-color += 10%" in context-menu.styl produces
an invalid color entry in CSS
As described in Automattic#713 - Automattic#713

node generates main.css out of the styles files on demand. This might be convenient for many scenarios, but this is a big security issue because node must have write access to its own code directory -- thus some bug might allow attacker to overwrite ones own code. IMO, Kue should include main.css in the repository, so that a locked-down server without write access wouldn't break it.
@nyurik nyurik closed this Sep 15, 2015
@nyurik nyurik reopened this Sep 15, 2015
@nyurik
Copy link
Author

nyurik commented Sep 15, 2015

@behrad, adding the main.css to the distribution solves the problem for the secure servers - in our case, Wikimedia servers are well secured, and their settings do not allow on-the-fly code modification - that's why I noticed this issue. In general case, I don't think stylus should generate code dynamically at all - its not a good practice to rely on it - code should stay static. As for injection - its impossible to "fix it" via the code here, it can only be solved by doing what we already did - by setting proper file permissions on the production server. This way if someone discovers a bug in our code and uses it to try to write a local file, they will fail because of the file permissions.

@nyurik
Copy link
Author

nyurik commented Sep 15, 2015

P.S. I do notice that once in a while the style does not load even though it is there, and I have to refresh. I don't know what's causing it, might be some other stale cache issue.

@behrad
Copy link
Collaborator

behrad commented Nov 19, 2015

Kue is updated, and there are conflicts :)
Can you rebase this PR @nyurik ?

@nyurik
Copy link
Author

nyurik commented Nov 19, 2015

@behrad , i rebased it -- see #755

@nyurik nyurik closed this Nov 19, 2015
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.

2 participants