Skip to content

fix: Round 1+2 framework hardening — security, reliability, performance, DX - #8

Merged
biglyan merged 6 commits into
masterfrom
claude/gifted-hawking-rnlk3i
Jun 25, 2026
Merged

fix: Round 1+2 framework hardening — security, reliability, performance, DX#8
biglyan merged 6 commits into
masterfrom
claude/gifted-hawking-rnlk3i

Conversation

@biglyan

@biglyan biglyan commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Three rounds of P0–P3 hardening across the PlumePHP v1.3.1 framework (236 tests, all green).

Round 1 — Foundation fixes

  • isMobile() operator-precedence bug (&& vs ||)
  • CSRF: enforce APP_SECRET env var (no silent fallback to a weak key)
  • authcode(): deprecation notice (MD5/RC4 not cryptographically safe)
  • composer.json: PSR-4 autoload path corrected
  • C(): null-byte sentinel for worker-mode config snapshot/restore (FrankenPHP/RoadRunner)
  • PlumeLogger: register_shutdown_function so logs aren't lost on fatal errors
  • PlumeParam: removed implicit htmlentities() on __get() — XSS escaping belongs in templates

Round 2 — Correctness & DX

  • ReflectionMethod PHP 8.4 deprecation fix in PlumeEvent
  • PlumeSchema constructor: removed return (PHP ignores constructor returns)
  • scripts/build.php: inline common.php into dist so the single-file build is self-contained
  • PlumeHelper::isFromBrowser(): removed static $retVal worker-mode cache
  • PlumeHelper::generateNonceStr(): mt_rand() → random_int()
  • PlumeHelper::signature(): E_USER_WARNING when default weak key is used
  • PlumeHelper::getCurrentUrl(): percent-encode non-ASCII/unsafe chars in REQUEST_URI
  • PlumeEngine::runAction(): redact password/token fields before logging; remove dead catch(\Exception) after catch(\Throwable)
  • PlumeResponse::cache(false): single-string Cache-Control (was an array, broke some clients)
  • PlumeSchema::jsonSerialize(): static $propMap[] cache to avoid repeated ReflectionClass per call
  • Typo: "Wellcome" → "Welcome" in CLI help
  • PlumeLoader::reset(): document why $dirs is intentionally not reset

Round 3 — Security, reliability, and robustness

P0

  • Action::error(): escape $msg with htmlspecialchars() before embedding in HTML (XSS)
  • Curl::postJson(): json_encode() array data, add missing curl_close(), add CURLOPT_CONNECTTIMEOUT
  • Logger::notice(): write to both .log (buffered) and .log.wf (immediate) per docs

P1

  • Request::getMethod(): _method tunnelling only permitted when REQUEST_METHOD=POST (prevents CSRF bypass via GET)
  • Router::saveCache(): atomic write via temp file + rename() — no partial-write corruption
  • Action::createCsrfCookie(): random_bytes(8) mask instead of str_shuffle() (CSPRNG)
  • Action::validateCsrfToken(): replace runtime mb_strlen(hash_hmac(...)) with the constant 64

P2

  • Route::matchMethod(): in_array() instead of array_intersect() (no temp array allocation per route check)
  • View::e(): htmlspecialchars(ENT_QUOTES|ENT_SUBSTITUTE, 'UTF-8') instead of bare htmlentities()
  • DotEnv::parseValue(): detect \t# as well as # for inline comments
  • Container::get(): throw PlumeContainerException when a binding resolves to abstract/interface
  • Param::__set(): widen type hint string → mixed (allows arrays, booleans, ints)
  • Logger::write(): random_int() instead of mt_rand() for log-ID entropy

P3

  • Action::render(): expose $csrf_token and $csrf_field template vars (canonical names per docs); legacy aliases kept
  • Action::run(): move Content-Type: text/html header to render() so JSON-only actions aren't affected
  • Curl: remove empty CURLOPT_USERAGENT / CURLOPT_REFERER setters

Test plan

  • vendor/bin/phpunit tests/ — 236 tests, 0 failures
  • New tests: NOTICE dual-log, _method GET guard, atomic cache write, e() HTML escaping, tab-comment .env, abstract-binding exception, mixed-type Param set

🤖 Generated with Claude Code

https://claude.ai/code/session_01EeyotN3pAMLiyrxo58MZyX

claude added 3 commits June 25, 2026 06:50
@claude
fix: implement full P0-P3 framework optimizations
2ff6dec 
P0 - Critical bugs:
- Fix isMobile() operator precedence: `or` replaced with `||` to prevent
  short-circuit return that caused the method to always evaluate only the
  first isset() and ignore UA matching
- Enforce APP_SECRET for CSRF protection: getCsrfKey() now throws
  RuntimeException when APP_SECRET is absent (non-testing env) or < 16
  chars, eliminating the predictable fallback key vulnerability

P1 - Production safety:
- Deprecate authcode(): adds @deprecated docblock and E_USER_DEPRECATED
  trigger pointing to sodium_crypto_secretbox/openssl_encrypt alternatives
- Fix composer.json PSR-4 path: corrects "src/" → "library/Plume/" so
  `Plume\` namespace resolves correctly after composer dump-autoload; adds
  "files" autoload entry for PlumePHP.php global functions
- Add C() snapshot/restore for worker-mode config isolation: sentinel keys
  (\x00snapshot_take\x00 / \x00snapshot_restore\x00) let boot() snapshot
  config after loading and resetForWorker() restore it, preventing
  per-request C() writes from leaking across FrankenPHP/RoadRunner requests

P2 - Performance and reliability:
- Logger: register_shutdown_function flushes buffered DEBUG/INFO/NOTICE
  logs on PHP fatal errors where __destruct() is not guaranteed to run
- runAction(): static $pathCache[] uses null-coalescing assignment ??= to
  cache file_exists() results for the worker process lifetime, eliminating
  repeated stat() syscalls on hot paths
- PlumeParam: remove implicit htmlentities() from __get(); add explicit
  html() method for safe HTML-context output; prevents data mangling for
  DB writes, numeric values, and API responses

P3 - Test coverage:
- RequestTest: 5 new isMobile() tests covering desktop UA, Android UA,
  HTTP_X_WAP_PROFILE, HTTP_PROFILE, and missing UA edge cases
- LoggerTest: testSaveIsIdempotentOnEmptyBuffer,
  testExplicitSaveFlushesBeforeDestruct
- CsrfTest: 3 new tests for APP_SECRET enforcement (missing, short, valid)
  + setUp/tearDown APP_SECRET lifecycle management
- ConfigWorkerTest (new): 5 tests for C() snapshot/restore contract
- ParamTest (new): 9 tests for PlumeParam raw/html access semantics

All 221 tests pass, 0 errors.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EeyotN3pAMLiyrxo58MZyX
@claude
chore: ignore library/storage/ runtime log directory
81a4009 
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EeyotN3pAMLiyrxo58MZyX
@claude
Round 2 P0-P3: correctness, security, and DX hardening
1903f29 
P0
- PlumeSchema::__construct: drop silently-ignored `return $mapper->map(...)` from constructor
- scripts/build.php: inline library/common.php into dist so global-function aliases are available in the single-file distribution

P1
- PlumeHelper::isFromBrowser: remove `static $retVal` — cached result leaked across requests in worker mode (FrankenPHP/RoadRunner)
- PlumeHelper::signature: emit E_USER_WARNING when called with the hardcoded default key
- PlumeHelper::generateNonceStr: replace mt_rand() with random_int() (CSPRNG)
- Engine::runAction: redact password/token/secret/cvv fields from $_REQUEST before writing to access log
- PlumeEvent::execute: fix deprecated single-string ReflectionMethod constructor (PHP 8.4)

P2
- Engine::_error: remove unreachable `catch(\Exception)` after `catch(\Throwable)` and drop PHP < 7 comment
- Response::cache(false): replace multi-value array (with IE6 post-check/pre-check directives) with a single RFC-compliant string
- PlumeSchema::jsonSerialize: cache property→snake_name map per class in a static array to avoid repeated ReflectionClass instantiation

P3
- PlumeCmdService::showWelcome: fix typo "Wellcome" → "Welcome"
- PlumeHelper::getCurrentUrl: percent-encode non-ASCII and URI-unsafe bytes in REQUEST_URI before appending
- PlumeLoader::reset: document why static $dirs is intentionally excluded from reset

Tests: 228 pass

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EeyotN3pAMLiyrxo58MZyX
@biglyan biglyan changed the title fix: P0-P3 framework optimizations — security, reliability, performance fix: Round 1+2 framework hardening — security, reliability, performance, DX Jun 25, 2026
@claude
Round 3 P0-P3: security, correctness, and robustness hardening
94986fc 
P0 – XSS / correctness:
- Action::error() escapes $msg with htmlspecialchars() before embedding in HTML
- Curl::postJson() json_encodes array data, adds missing curl_close(), adds
  CURLOPT_CONNECTTIMEOUT
- Logger::notice() now writes to both .log (buffered) and .log.wf (immediate),
  consistent with docs

P1 – security / reliability:
- Request::getMethod(): _method override only applies when REQUEST_METHOD=POST
  (prevents CSRF-bypass via GET tunnelling)
- Router::saveCache(): atomic write via temp-file + rename() so a partial write
  never corrupts the cache
- Action::createCsrfCookie(): replace str_shuffle() with random_bytes(8) for
  cryptographically secure CSRF mask
- Action::validateCsrfToken(): SHA-256 hex length is always 64 — remove the
  needless hash_hmac() call that computed it dynamically

P2 – performance / correctness:
- Route::matchMethod(): in_array() instead of array_intersect() — avoids
  allocating a temp array on every route check
- View::e(): htmlspecialchars(ENT_QUOTES|ENT_SUBSTITUTE, UTF-8) instead of
  bare htmlentities() with no charset
- DotEnv::parseValue(): detect tab+# as well as space+# for inline comments
- Container::get(): throw PlumeContainerException when a binding resolves to an
  abstract class or interface instead of crashing with a fatal error
- Param::__set(): widen type hint from string to mixed so arrays/booleans/ints
  can be stored without a TypeError
- Logger::write(): random_int() instead of mt_rand() for log-ID entropy

P3 – DX / alignment with docs:
- Action::render() sets csrf_token and csrf_field template vars (canonical names
  from CLAUDE.md); legacy csrfToken / _csrf_form_ kept for backward compat
- Action::run() moves Content-Type header to render() so JSON-only actions are
  not forced into text/html
- Curl: remove empty CURLOPT_USERAGENT / CURLOPT_REFERER setters

Tests: 236 pass (+8 new covering notice dual-log, _method GET guard, atomic
cache write, e() escaping, tab-comment dotenv, abstract-binding throw,
mixed-type param set)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EeyotN3pAMLiyrxo58MZyX
@biglyan
biglyan marked this pull request as ready for review June 25, 2026 10:51
claude added 2 commits June 25, 2026 10:53
@claude
Action::render(): drop legacy CSRF template-var aliases
15ed732 
Remove csrfToken, csrfPost, and _csrf_form_ — no usage exists outside
library/. Only the canonical csrf_token and csrf_field (per CLAUDE.md)
are now injected.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EeyotN3pAMLiyrxo58MZyX
@claude
signature(): remove default key, make $key a required argument
7acf869 
The default 'afjd32t4-…' key and the trigger_error() guard around it
are deleted. Callers must now supply their own key — PHP enforces this
with a TypeError at call sites that omit it. The wrapper in common.php
and the copy in dist/Plume.php are updated in lock-step.

Test updated: expectWarning → expectException(\TypeError::class).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EeyotN3pAMLiyrxo58MZyX
@biglyan
biglyan merged commit ece6946 into master Jun 25, 2026
6 checks passed
@biglyan
biglyan deleted the claude/gifted-hawking-rnlk3i branch June 25, 2026 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@biglyan @claude