Fixes T3922. Add a linter which complains if files are executable when they shoudn't be.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- Restricted Maniphest Task
- Commits
- rARC904fbe737ce5: Add an `ArcanistChmodLinter`.
Tested by modifying the .arclint file of the rARC repository and running arc lint --everything.
Diff Detail
Diff Detail
- Repository
- rARC Arcanist
- Branch
- chmod-linter
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 559 Build 559: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
| src/lint/linter/ArcanistChmodLinter.php | ||
|---|---|---|
| 49 | Maybe we want to make a Filesystem::isExecutable function to use here. | |
| 79–88 | This could be moved to libphutil possibly. | |
| 108–115 | This list is probably incomplete... Also it seems strange to me that text/plain is required, but this is apparently the MIME type of ./bin/arc, which is obviously a valid executable. Furthermore, in rP, bin/aphlict and support/aphlict/server/aphlict_launcher.php are both detected as being of MIME type 'text/x-c, which seems incorrect. Possible we could have some magic comment (like the vim modeline) which tells us what type of file this is. | |
Comment Actions
Hmm, I worry that the getScriptMimeTypes() stuff is going to have like a billion false positives for all time, and generally that MIME type detection won't work well here.
What do you think about this instead?
- Don't try to detect MIME types.
- Assume real executables (binaries) will be covered by shouldLintBinaryFiles() already.
- Just check for shebang on the first line of executable files.
- If it's not present, raise "Unnecessary +x or missing shebang."
Comment Actions
- Don't try to detect MIME types.
- Assume real executables (binaries) will be covered by shouldLintBinaryFiles() already.
- Just check for shebang on the first line of executable files.
Comment Actions
Closed by commit rARC904fbe737ce5 (authored by @joshuaspence, committed by @epriestley).