Repository navigation
💥 feat: generated files leave the plugin folder (3.0) - #128
Merged
Merged
Conversation
Audit S6/S7. Up to 2.x compiled Blade views (<plugin>/.cache, .bladec) and log files (<plugin>/storage/logs) were written inside the plugin folder, created 0777, where the web server serves them as text; and "errorlog", the default, also wrote a daily file there, against its own documentation. - Support\Storage::path(plugin, folder): uploads/wpbones/<plugin folder>/ <folder>, made with wp_mkdir_p, an index.php at every level and a deny-all .htaccess at the top (left alone once it exists); the temp folder when uploads cannot be written. Named after the plugin's folder: the slug is read from the header on init, after the logger is made. - Blade is made when a Blade view first renders, compiles there, as .php. - Logs: "errorlog" is error_log() only; "single" and "daily" default to Storage, with a hash keyed by AUTH_SALT in the file name (not wp_hash(), which is pluggable and not loaded yet at boot); a plugin.logging.path of the plugin's own keeps its folder and names.
- no temporary-folder fallback: a shared /tmp would let other local users read the logs and plant compiled views. When uploads cannot be written, Storage::path() answers null: a Blade view throws a RuntimeException that says why, a single/daily log goes to error_log() only; - a deny rule that cannot be written means no folder, as Apache would serve what it holds; - the test no longer removes a shared /tmp/wpbones; - View::render(true) closes its buffer when the view throws.
- an .htaccess is accepted only if it denies: one that a short write left empty or partial (a full disk) is removed, and an existing one that denies nothing means no folder; - View::render(true) restores the buffer level it found, so a view that throws inside a Blade @section or @Push no longer leaves render()'s own buffer open.
- a folder another request made first is used (wp_mkdir_p() answers false when it loses the race), and the .htaccess is written aside and renamed, so no request reads it half written; - `Require all denied` alone (Apache 2.4) is accepted as the rule; - the log name is keyed with AUTH_SALT only (or the salt WordPress keeps in the options): ABSPATH differed between a web request and WP-CLI, and between the releases of an atomic deploy; - Blade runs with throwOnError: BladeOne's showError() closed a buffer it had not opened, the page's own; - Storage::delete($plugin), for an uninstall.php; - tests: the log key with a real AUTH_SALT, in a process of its own (the suite never defined one, so dropping the salt passed).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For 3.0, on
v3: the files a plugin generates leave the plugin folder. This is the breaking half of audit S6/S7. 2.1.3 (#126) shipped what 2.x could fix in place.💥 Where they go
Support\Storage::path($plugin, $folder)returnsuploads/wpbones/<plugin folder>/<folder>.wp_mkdir_p(), with anindex.phpat every level and a deny-all.htaccessat the top.path()answersnull:RuntimeExceptionthat says why;singleordailylog falls back toerror_log()./tmpwould let other local users read the logs and plant compiled views (Codex)..htaccess:Deny from allorRequire all denied).Storage::delete($plugin)is there for anuninstall.php.init. The first live run, with the slug, wrote intowpbones/plugin/.<plugin>/.cache/*.php, the folder made by everyViewuploads/wpbones/<plugin>/views/*.php, made only when a Blade view first renderssingle/dailylogs, default path<plugin>/storage/logs/debug.log,<date>.loguploads/wpbones/<plugin>/logs/debug-<hash>.log,<date>-<hash>.logerrorlogerror_log()onlyplugin.logging.pathwp_mkdir_p())The log file name carries a 12-character HMAC keyed with
AUTH_SALTonly.ABSPATHis not in the key: it differs between web requests and WP-CLI, and between the releases of an atomic deploy. The HMAC makes the name impossible to guess where the.htaccessis not read (nginx). The key is notwp_hash(): it is pluggable and not loaded when the plugin boots. A Brain Monkey stub hid that; the live run fataled.Tests
composer test: 317 tests and 842 assertions, against 307 and 824 onv3.StorageTest,ViewBladeCacheTest,LogServiceProviderTest.GeneratedFilesTest(2.1.3) keeps only itserrorloghalf; the rest moved.Live, in WPKirk-Developing, with
storage-live-smoke.sh, on a 3.0 checkout (11 checks):logger()call writes no file;.bladecand no<plugin>/.cache;uploads/wpbones/WPKirk-Developing/views, and a direct request for it shows no PHP;.htaccessis present;singlelog lands indebug-<hash>.logwith its message.11 ✓. The same smoke on v2.1.2 fails 3 of its 6 common checks.
Review
/tmp/wpbones..htaccesspassed for a rule, andrender()left its buffer open with a Blade@sectionopen.Require all deniedform;ABSPATHin the HMAC key;showError()closing the page's buffer;AUTH_SALT.ms-files.phpnetworks and remote uploads (S3) are not protected by the.htaccess.composer test: 327 tests and 857 assertions;storage-live-smoke.sh: 11 ✓.