Skip to content

fix: code quality cleanup — DI, dedup, CSP, reflection, dead code#100

Open
rhoerr wants to merge 4 commits intovpietri:mainfrom
rhoerr:qdb/pr7-cleanup
Open

fix: code quality cleanup — DI, dedup, CSP, reflection, dead code#100
rhoerr wants to merge 4 commits intovpietri:mainfrom
rhoerr:qdb/pr7-cleanup

Conversation

@rhoerr
Copy link
Copy Markdown

@rhoerr rhoerr commented Apr 5, 2026

Summary

  • Replace ObjectManager usage with constructor DI in Toolbar and Panel blocks
  • Inject SerializerInterface in Helper/Data, fix and&& operators
  • Hash session IDs in temp filenames, add cleanup in shutdown path
  • Fix cache dedup bug ($cacheEvents$this->cacheEvents)
  • Remove static CSP script-src hashes (SecureRenderer handles nonces)
  • Guard reflection calls with try/catch \ReflectionException
  • Remove dead Response plugin
  • Fix Elasticsearch::pullData() return type
  • Scope window.fetch monkey-patch in dumper.phtml

Security Findings Addressed

ID Finding Severity
M7 ObjectManager usage instead of DI Medium
M8 Raw session ID in temp filenames Medium
M9 Static CSP hashes are fragile Medium
H10 Cache dedup bug (missing $this->) Hygiene
H11 Unguarded reflection calls Hygiene
H12 Dead Response plugin Hygiene

Test plan

  • Verify toolbar base URL still resolves correctly
  • Verify panel tab AJAX URLs still work
  • Verify config serialization still works
  • Verify temp files use hashed names
  • Verify cache tab shows deduplicated entries
  • Verify CSP does not block toolbar scripts
  • Verify plugin tab and layout block tree still render
  • Verify dumper tab fetch interception still works

🤖 Generated with Claude Code

rhoerr and others added 3 commits April 5, 2026 18:27
- Replace ObjectManager usage with constructor DI in Toolbar and Panel
- Inject SerializerInterface in Helper/Data, fix and→&& operators
- Hash session IDs in temp filenames, add cleanup in shutdown path
- Fix cache dedup bug ($cacheEvents → $this->cacheEvents)
- Migrate static CSP hashes to SecureRenderer nonces
- Guard reflection calls with try/catch \ReflectionException
- Remove dead Response plugin
- Fix Elasticsearch pullData() return type
- Scope window.fetch monkey-patch in dumper.phtml

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Without explicit argument override, UrlInterface resolves to
Backend\Model\Url in the adminhtml area, breaking frontend URLs
generated by Toolbar and Panel blocks.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The method indiscriminately deletes temp files older than 20s,
which can destroy other sessions' data. It was never called before
PR7 added it, and the TODO in cleanOldFiles() itself notes it
needs scoping to the current session before use.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@rhoerr rhoerr marked this pull request as ready for review April 6, 2026 01:57
Panel gained a UrlInterface $frontUrl constructor parameter when
ObjectManager::get() was replaced with proper DI injection. Six
subclasses that override the constructor were not updated to accept
and forward this parameter, causing DI compilation errors:
"Incompatible argument type: Required UrlInterface. Actual: array"

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.

1 participant