Skip to content

Add browser-scoped request history browser - #23

Merged
niden merged 15 commits into
phalcon:masterfrom
Alistar84:feature/request-history
Sep 26, 2026
Merged

niden merged 15 commits into
phalcon:masterfrom
Alistar84:feature/request-history

Conversation

@Alistar84

@Alistar84 Alistar84 commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

Hello!

In raising this pull request, I confirm the following:

  • I have read and understood the Contributing Guidelines
  • I have checked that another pull request for this purpose does not exist
  • I wrote tests for this PR
  • I have updated the relevant CHANGELOG
  • I have created a separate PR for the documentation (the documentation is included in this PR)

Small description of change:

This PR adds an optional, browser-scoped request history system to the debug bar and integrates it with compact request metrics in the bottom bar.

It resolves #21, which describes the current limitation where the bar can only inspect the request that rendered the current page. Completed AJAX requests, redirects, and earlier requests can now be inspected without navigating away from or reloading the host page.

Closes #21.

History browser

When history is enabled, each stored entry contains the collected debug-bar payload and request metadata such as the HTTP method, URI, response status, AJAX flag, and request and persistence timestamps.

The rightmost request control combines a search icon with the HTTP method and URI. It identifies the current request when the page first loads and updates when a stored request is selected. Clicking it closes any open collector panel and opens the history browser. Selecting a request replaces the complete bar payload and reopens the collector panel that was active before History.

Time and Memory are enabled by default and render once as compact, clickable indicators. Each collector owns its indicator metadata, and the indicators remain available when History is disabled.

Browser identity, storage, and retention

History is disabled by default and requires an explicit absolute writable storage path. The provider creates the directory when needed and validates its writability during boot. History can also be disabled through collectors.history.

History uses a dedicated random phalcon-debugbar-history cookie and never reads, starts, or changes the application's PHP session. The cookie is HttpOnly, SameSite=Lax, marked Secure on HTTPS, scoped to the application's base path, and lasts for the browser session. It is appended through PHP's native cookie mechanism so it does not replace cookies queued by the application.

The first allowed response sets the cookie but is not stored. Storage starts with the next request carrying that cookie, so clients that do not retain cookies create no history directories. Closing the browser discards its identity; associated files remain eligible for cleanup until history.ttl_seconds expires.

Stored requests are isolated by a SHA-256 hash of the browser identity, written atomically, limited per browser by history.max_requests, and removed after the configured TTL. The total browser-directory count is bounded by history.max_browsers (default 10). Creation is serialized with a filesystem lock and evicts the least recently updated browser directory before admitting another identity, bounding forged or repeatedly rotated cookie identities. Versioned payloads and metadata sidecars keep listings independent of collector payload size. Rate-limited garbage collection removes expired entries, abandoned temporary files, and empty browser directories.

Internal endpoint

The provider exposes its package-owned controller at:

  • GET /_debugbar/open - list stored request metadata
  • GET /_debugbar/open?id=<request-id> - load a stored debug-bar payload
  • DELETE /_debugbar/open - clear the current browser's history

The endpoint resolves the final application base URI at request time without adding routes or resolving shared responses during boot. It reuses the debug bar access gate, returns private non-cacheable JSON responses with X-Content-Type-Options: nosniff, validates request IDs, accepts only the actual transport method, and is excluded from debug-bar collection, headers, injection, and storage. Applications with dispatcher ACL plugins can allow the public HistoryEndpoint::CONTROLLER_NAMESPACE.

Backward compatibility

The feature is opt-in. Existing applications retain the current behavior when History is disabled. Time and Memory remain enabled by default and can be disabled through the collector configuration.

Tests

The PR includes PHP and JavaScript tests covering browser-cookie isolation, cookie attributes and access gating, Phalcon 5 and 6 endpoint dispatch, storage failure paths, concurrent directory creation, global browser eviction, retention, Windows glob paths, version compatibility, stale asynchronous responses, CommonJS host isolation, metadata-driven request indicators, and stored-request selection. The PHP suite has 100% statement and method coverage.

Thanks

@niden

niden commented Sep 2, 2026

Copy link
Copy Markdown
Member

@Alistar84 At some point please rebase from master. I put a fix in for the warning emitted in the CI that makes this CI red. Thanks

@Alistar84
Alistar84 force-pushed the feature/request-history branch from d26cba8 to 603fa1c Compare September 2, 2026 15:07
@Alistar84

Copy link
Copy Markdown
Contributor Author

Rebased onto the latest master. Thanks for the fix!

@Alistar84
Alistar84 marked this pull request as ready for review September 3, 2026 06:38
@niden

niden commented Sep 7, 2026

Copy link
Copy Markdown
Member

@Alistar84 Can we look in this one when you can? You will need to merge master. I am guessing the coverage dropped for this one and also I introduced new php-cs-fixer rules so run it when you can over here.

Alistar84 and others added 7 commits September 9, 2026 14:27
Assisted-by: Codex
Harden session storage, request handling, and asynchronous history controls while preserving legacy entries and full test coverage.

Assisted-by: Codex
Assisted-by: Codex
@Alistar84
Alistar84 force-pushed the feature/request-history branch from 603fa1c to 11b8040 Compare September 10, 2026 07:41
@Alistar84

Copy link
Copy Markdown
Contributor Author

Hi @niden, I've updated the PR following your comments about merging the latest master, restoring coverage, and running the updated PHP CS Fixer rules.

  • The branch is based on the current upstream/master (81f4583).
  • Composer validation, PHPCS, PHPStan, and PHP CS Fixer all pass.
  • PHP: 286 tests, 821 assertions.
  • JavaScript: 17 tests pass.
  • Coverage: 100% (1627/1627 statements and 279/279 methods).
  • History remains opt-in; when disabled, the existing Time and Request tabs and collector set are preserved.

Co-authorship disclosure: these changes were co-authored with Codex (OpenAI), which assisted with implementation, tests, and review.

@niden

niden commented Sep 10, 2026

Copy link
Copy Markdown
Member

@Alistar84 Thank you for this. I will check it out later tonight. It looks good at first glance but I have noticed a couple of things I need to check locally first before posting. More to come later on.

@niden niden left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments listed inline.

This is really good work and the main mechanics are in place. Have a look at the comments and share your thoughts on them.

Comment thread src/DebugBar/ResponseListener.php Outdated
Comment thread src/DebugBar/Provider.php
Comment thread resources/assets/debugbar.js Outdated
Comment thread src/DebugBar/History/FilesystemHistory.php Outdated
Comment thread src/DebugBar/History/FilesystemHistory.php Outdated
Comment thread resources/assets/debugbar.js Outdated
Comment thread resources/assets/debugbar.js
Comment thread src/DebugBar/Provider.php Outdated
Comment thread src/DebugBar/History/FilesystemHistory.php
Comment thread src/DebugBar/History/HistoryOptions.php Outdated
Gabriele Propersi and others added 2 commits September 14, 2026 08:56
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
@Alistar84

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review. I have gone through all the comments and pushed the related changes.

The main updates are the package-owned History endpoint, the History and filesystem contracts, versioned stored entries, PHP-driven widget metadata, independent collector settings, and the required storage path.

Local checks are green: 297 PHP tests, 18 JavaScript tests, PHPStan, PHPCS, and PHP CS Fixer across all 164 files.

Both new commits also include the requested Co-authored-by: Codex codex@openai.com trailer.

@Alistar84

Copy link
Copy Markdown
Contributor Author

@niden The requested changes and inline replies are now in place, and the CI is green. When you have a chance, could you please take another look? Thanks!

@niden

niden commented Sep 17, 2026

Copy link
Copy Markdown
Member

@niden The requested changes and inline replies are now in place, and the CI is green. When you have a chance, could you please take another look? Thanks!

@Alistar84 Thank you so much for all this work. First glance it looks great. I will go for rournd 2 later on today.

@niden niden left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some additional findings, none on the core functionality.

We will also need a revisit on the description and documentation.

Also, consider its usage. Is it on by default? If so, we need to document that.

Comment thread src/DebugBar/Provider.php
Comment thread src/DebugBar/Provider.php Outdated
Comment thread src/DebugBar/History/NativeHistoryFileOperations.php
Comment thread src/DebugBar/History/HistoryOptions.php Outdated
Comment thread src/DebugBar/Collector/HistoryCollector.php Outdated
Comment thread resources/assets/debugbar.js Outdated
Comment thread src/DebugBar/Controllers/HistoryController.php Outdated
Comment thread src/DebugBar/History/FilesystemHistory.php Outdated
Comment thread src/DebugBar/History/FilesystemHistory.php
Comment thread tests/JavaScript/support/dom.js
Co-authored-by: Codex <codex@openai.com>
@Alistar84

Copy link
Copy Markdown
Contributor Author

@niden I pushed 37f347d and went through all the latest review points.

The main changes are: validation now happens after the runtime gates, collectors.history is honored, storage paths and glob metacharacters are handled safely, session-directory writability is rechecked, payload publication comes before metadata, and destructive dispatch uses the real transport method. I also moved method/URI to semantic Request metrics and made Time/Memory own their indicator metadata, so History is no longer coupled to them.

I refreshed the PR description/docs and added regression coverage for each of those cases. Local checks are green: 305 PHP tests (902 assertions), 18 JavaScript tests, PHPCS, PHP CS Fixer, and targeted PHPStan on the changed PHP files.

Ready for another look when you have time - thanks!

Co-authored-by: Codex <codex@openai.com>
@Alistar84

Copy link
Copy Markdown
Contributor Author

Small follow-up in f75a04a: CI caught three uncovered lines under the repository's 100% coverage gate. I covered the disabled/root path cases (including UNC) and removed an unreachable fallback around a constant valid regex. Targeted tests, PHPStan, and style checks are green.

@niden

niden commented Sep 18, 2026

Copy link
Copy Markdown
Member

@Alistar84 once more thank you for this excellent work.

Since we are close to the completion of this feature, I wanted to really see how this looks like. So I pulled down your branch on my local repo. Then I fired up the Vokuro application, which has the debugbar enabled by default. I copied the code from your branch in the vendor/ folder of Vokuro to see how it looks and behaves.

With that and with clicking around plus an additional AI review we have a few things to address.

This is how the bar shows with history on:

image

As you can see we have some inconsistencies there with time/memory and it is a bit tall. I took the liberty of changing a few things (diff in a follow up comment for your convenience), and it should look like this

image

Note that in the second picture the history does not show (I disabled it) but it will appear next to the memory. This shortens the height of the bar and removes the duplicate time/memory, thus allowing for a less cluttered UI.

The findings are in the next comment

@niden

niden commented Sep 18, 2026 •

Copy link
Copy Markdown
Member

1. History breaks app routes (boot-time service lookup)

Provider::registerHistory() calls getShared('url') and getShared('response') in boot(). Phalcon caches the first shared instance, and Di::set() does not clear that cache. When the app registers its own url after the bar boots, the app keeps the default Url, which guesses the base URI from PHP_SELF.

With History on, /session/login and /session/signup fail with "LoginController / SignupController handler class cannot be loaded"; the endpoint fails with "OpenController". With History off, all pages load.

Resolve at request time.

  • boot() resolves no services for History. The endpoint keeps the container and reads the url service on first use (in the application:beforeHandleRequest listener or at collect time). All app providers and module registerServices() have run by then, so the bar reads the app's final url and changes nothing.
  • Use only the path of the base URI (parse_url(..., PHP_URL_PATH)). This also covers item 3 (absolute base URI).
  • HistoryController creates its own Phalcon\Http\Response (or reads response from the DI when the action runs). No boot-time getShared('response').
  • No boot-order rule for users.

NOTE: Request does the same thing but has not been observed until now, since it is more common to use URI than replace Request. That will be a follow up PR - not in scope now.

2. The endpoint does not start the session

Most Phalcon apps start the session lazily, when code resolves the session service. On /_debugbar/open no app code does that, so session_status() is not active and find()/get() return empty results, even though the browser sends the session cookie and the entries are on disk.

The page was stored in its session directory, but GET /_debugbar/open with the same cookie returned {"requests":[]}. With $di->get('session') in the bootstrap (temporary test), the list and the detail worked.

The bar uses its own cookie, not the PHP session.

  • We cannot interfere with an application i.e. start the session ourselves. The bar needs to be as least intrusive as possible. As such, un the first recorded response, the bar sets its own random ID cookie (HttpOnly, SameSite=Lax, path from the base URI). History uses a hash of that ID as the storage key, in place of hash('sha256', session_id()).
  • The bar never reads, starts, or changes the app's session.
  • The cookie is set only when the environment and access gates allow the bar.
  • Works for apps without a session.
  • Also fixes 9: session_regenerate_id() at login no longer hides earlier entries.
  • Docs and PR description: change "isolated by session" to "isolated by browser (debug bar cookie)".

3. An absolute base URI breaks the endpoint

With an absolute base URI (for example http://localhost:8080/), the endpoint URL becomes a full URL. HistoryEndpoint::matches() compares the request path with that full URL and never matches. The app router then handles the request.

The JS gets http://localhost:8080/_debugbar/open; GET /_debugbar/open throws "DebugbarController handler class cannot be loaded".

Covered by item 1 (use only the path of the base URI), plus validation.

  • HistoryOptions::validate() also checks history.url: it must start with / and have no scheme, host, query, or fragment. Otherwise throw a clear InvalidArgumentException, the same as the history.path checks.

4. Dispatcher ACL plugins can block the endpoint

The endpoint runs through the app's normal dispatch loop. Plugins on dispatch:beforeDispatch / dispatch:beforeExecuteRoute (for example INVO's SecurityPlugin) see controller history, action open, and can forward, redirect, or deny.

A dispatcher listener saw history:open, returned false, and the endpoint answered 401 Unauthorized. Vokuro itself is not affected (ACL lives in ControllerBase; HistoryController does not extend it).

Documentation plus a public constant.

  • Expose the controller namespace as a constant, for example HistoryEndpoint::CONTROLLER_NAMESPACE, and use it in HistoryEndpoint::__invoke().
  • Docs show how an ACL plugin allows the endpoint: if (HistoryEndpoint::CONTROLLER_NAMESPACE === $dispatcher->getNamespaceName()) { return true; }
  • The bar injected into the 401 response is handled in item 11.

5. Indicators depend on History

Time and Memory show as a tab and as an indicator. The right side (two indicators + request control up to 40vw) squeezes the tabs, which scroll. The indicators show only when History is on (coupling Time/Memory to History). collectors.time = false removes both the tab and the indicator.

Diff for the change in the next comment

  • A collector that declares widget.indicator gets no tab; it shows one time as an indicator button that opens/closes its panel (purple when active).
  • Indicators show with History on or off. History owns only the request control.
  • Request control max-width: 25vw (was 40vw); full label in the tooltip.
  • We will need JS tests: indicator opens/closes its panel; indicators render with History off; no tab for a collector with indicator, "request metrics render on the right alongside time and memory tabs"; "history-disabled payload retains the existing collector tabs".
  • Docs/CHANGELOG need an update - behavior change (no 6)

6. CHANGELOG line for the "Logs" tab name

7. Memory/Time are on by default

They should stay on by default and we should just document this

8. No global limit on disk use

max_requests is per storage key. A client without a cookie gets a new key on each request, so each request makes a new directory, kept for ttl_seconds (default 24 hours). Crawlers, curl, monitoring, and load-balancer health checks do this (a health check every 5 seconds is about 17,000 directories per day).

Storage strategy:

  • Store only when the request already carries the bar cookie (item 2). The first response sets the cookie; storing starts with the next request from that browser. Clients without cookies never create directories. Cost: the first page in a new browser is not stored.
  • Use access.allow_ips or access.callback when History is on for a host that other users can reach. <- This can be a follow up

9. A new session ID hides earlier entries

The storage key was hash('sha256', session_id()), so session_regenerate_id() (usually at login) moved the user to a new directory and earlier entries disappeared. Not reproduced in vokuro (no ID regeneration).

Covered by 2

10. Dead preferredActive code

On selecting a stored request, the code tries to reopen the panel that was open before (activeBeforeSelection ->
preferredActive) (debugbar.js:651-654, 763-770. Opening History calls closePanel() first (active = null), and opening a panel closes History, so active is always null at selection time. The path never runs.

This needs a second look

  • The History trigger saves active before it calls closePanel(); the selection uses the saved value as preferredActive.

  • Example: Database panel open -> open History -> pick another request -> that request's Database panel opens. Works for indicator panels too (item 5).

  • Of course test for this :)

11. Endpoint requests are treated like normal pages

(ResponseListener.php:64-78): for /_debugbar/open the listener skips only the storing. It still runs a full collect(), adds X-Debug-Bar (vokuro JSON response: X-Debug-Bar: 13), and can inject the whole bar. In the item 4 test, the ACL's 401 Unauthorized response had no Content-Type, fetch() sends no X-Requested-With, so the bar was injected into it.

Return early

  • Top of ResponseListener::__invoke(): if the endpoint matches, return. No collect, no header, no injection, no storing. The check in record() is then not needed.

12. Wrong panel-type lists in the docs

Review the docs and correct the lists so as to be accurate.

  • grid: version, request, config, session, route, memory
  • list: time, messages, database, view, cache
  • Add: "Time and Memory open their panels from the indicators on the right."

13. Small cleanups

Adding these here for further cleanup. AI found all of them to be honest, but I have verified them and they are accurate

  1. Provider.php:169: the ?? $this->historyOptions->url fallback can never run (the endpoint always exists when History is on).
  2. HistoryController.php:92-106: handle() copies properties to locals and passes them to closures as parameters; the closures can use $this.
  3. HistoryOptions.php:25: docblock says listener, collector, and controller share the class; only Provider and FilesystemHistory use it.
  4. debugbar.css:128/150, 292/297, 319/324: three selectors defined two times each; merge them.
  5. FilesystemHistory.php:118,296, HistoryEntry.php: import SORT_STRING and DATE_ATOM with use const, like the JSON constants.
  6. HistoryEntry.php: metadata() must come before metadataFromArray() (alphabetical method order).
  7. MemoryCollector.php: protected string $panel = 'grid' repeats the AbstractCollector default.
  8. Tests (for example FilesystemHistoryTest.php:285-287, 683-689): no @unlink / @rmdir, same rule as src.
  9. CHANGELOG.md: make the History entry short, user-facing bullets.

@niden

niden commented Sep 18, 2026

Copy link
Copy Markdown
Member

Diff as promised:

  • The bar gets a horizontal scrollbar. The tabs area has overflow-x: auto. The PR adds two indicators and a request control (up to max-width: 40vw) on the right, so the tabs get less width and scroll.

  • Time and Memory show two times: as a tab and as an indicator.

  • The indicators show only when History is on (if (historyPanel) in renderData). This couples Time/Memory to History.

  • A user cannot remove only the tab or only the indicator. collectors.time = false removes both.

  • History on: tabs on the left without Time/Memory. On the right: clock, gear, request control, then the toggle.

  • History off: the same, but without the request control.

Diff against the PR branch (f75a04a)

diff --git a/resources/assets/debugbar.js b/resources/assets/debugbar.js
--- a/resources/assets/debugbar.js
+++ b/resources/assets/debugbar.js
@@ -87,7 +87,8 @@
     }
 
     function metricIndicator(icon, label, value) {
-        var indicator = el('span', 'phalcon-debugbar-indicator');
+        var indicator = el('button', 'phalcon-debugbar-indicator phalcon-debugbar-metric');
+        indicator.type = 'button';
         indicator.title = label;
         indicator.appendChild(indicatorIcon(icon));
         indicator.appendChild(el('span', 'phalcon-debugbar-indicator-value', scalar(value)));
@@ -106,25 +107,29 @@
         return value;
     }
 
-    function renderIndicators(mount, data, widgets, onHistoryToggle, historyOpen, requestMetadata) {
+    function renderIndicators(mount, data, widgets, onHistoryToggle, historyOpen, requestMetadata, bindPanel) {
         mount.innerHTML = '';
         var historyTrigger = null;
 
         Object.keys(widgets).forEach(function (collector) {
             var definition = (widgets[collector] || {}).indicator;
-            if (!definition) {
+            if (!definition || !data[collector]) {
                 return;
             }
             var value = valueAtPath(data[collector], definition.path);
-            if (hasBadge(value)) {
-                mount.appendChild(metricIndicator(
-                    scalar(definition.icon),
-                    scalar(definition.label),
-                    value
-                ));
-            }
+            var indicator = metricIndicator(
+                scalar(definition.icon),
+                scalar(definition.label),
+                hasBadge(value) ? value : scalar(widgets[collector].label)
+            );
+            bindPanel(collector, indicator);
+            mount.appendChild(indicator);
         });
 
+        if (!onHistoryToggle) {
+            return historyTrigger;
+        }
+
         var request = data.request || {};
         var requestMetrics = request.metrics || {};
         var method = scalar(requestMetrics.method);
@@ -631,7 +636,7 @@
         function closePanel() {
             body.style.display = 'none';
             active = null;
-            Array.prototype.forEach.call(tabs.querySelectorAll('[data-panel-tab]'), function (child) {
+            Array.prototype.forEach.call(row.querySelectorAll('[data-panel-tab]'), function (child) {
                 child.classList.remove('is-active');
             });
         }
@@ -733,60 +738,63 @@
             }
 
             var preferred = null;
+
+            function bindPanel(name, node) {
+                var entry = data[name] || {};
+                var widget = widgets[name] || {};
+                var type = widget.panel || inferType(entry.panel);
+
+                node.setAttribute('data-panel-tab', name);
+                node.addEventListener('click', function () {
+                    setHistoryOpen(false);
+                    if (active === name) {
+                        closePanel();
+                        return;
+                    }
+                    activate(name, node, entry, type);
+                });
+                if (name === preferredActive) {
+                    preferred = [name, node, entry, type];
+                }
+            }
+
             Object.keys(data).forEach(function (name) {
                 var entry = data[name] || {};
                 var widget = widgets[name] || {};
-                if (widget.panel === 'history') {
+                if (widget.panel === 'history' || widget.indicator) {
                     return;
                 }
                 var label = widget.label || titleize(name);
-                var type = widget.panel || inferType(entry.panel);
 
                 var tab = el('button', 'phalcon-debugbar-tab');
                 tab.type = 'button';
-                tab.setAttribute('data-panel-tab', name);
                 tab.appendChild(el('span', 'phalcon-debugbar-tab-label', label));
                 if (hasBadge(entry.badge)) {
                     tab.appendChild(el('span', 'phalcon-debugbar-badge', scalar(entry.badge)));
                 }
 
-                tab.addEventListener('click', function () {
-                    setHistoryOpen(false);
-                    if (active === name) {
-                        closePanel();
-                        return;
-                    }
-                    activate(name, tab, entry, type);
-                });
-
+                bindPanel(name, tab);
                 tabs.appendChild(tab);
-                if (name === preferredActive) {
-                    preferred = [name, tab, entry, type];
-                }
             });
 
+            historyTrigger = renderIndicators(
+                indicators,
+                data,
+                widgets,
+                historyPanel ? function () {
+                    if (!historyOpen) {
+                        closePanel();
+                    }
+                    setHistoryOpen(!historyOpen);
+                } : null,
+                historyOpen,
+                requestMetadata,
+                bindPanel
+            );
+
             if (preferred) {
                 activate(preferred[0], preferred[1], preferred[2], preferred[3]);
             }
-
-            if (historyPanel) {
-                historyTrigger = renderIndicators(
-                    indicators,
-                    data,
-                    widgets,
-                    function () {
-                        if (!historyOpen) {
-                            closePanel();
-                        }
-                        setHistoryOpen(!historyOpen);
-                    },
-                    historyOpen,
-                    requestMetadata
-                );
-            } else {
-                indicators.innerHTML = '';
-                historyTrigger = null;
-            }
             setHistoryOpen(historyOpen);
         }
 
diff --git a/resources/assets/debugbar.css b/resources/assets/debugbar.css
--- a/resources/assets/debugbar.css
+++ b/resources/assets/debugbar.css
@@ -131,7 +131,7 @@
 }
 
 #phalcon-debugbar .phalcon-debugbar-request-control {
-    max-width: 40vw;
+    max-width: 25vw;
 }
 
 #phalcon-debugbar button.phalcon-debugbar-request-control {
@@ -147,6 +147,26 @@
     background: #26263a;
 }
 
+#phalcon-debugbar button.phalcon-debugbar-metric {
+    background: transparent;
+    border: 0;
+    border-left: 1px solid #2b2b40;
+    font: inherit;
+    cursor: pointer;
+}
+
+#phalcon-debugbar button.phalcon-debugbar-metric:hover {
+    background: #26263a;
+}
+
+#phalcon-debugbar button.phalcon-debugbar-metric.is-active {
+    background: #7c3aed;
+}
+
+#phalcon-debugbar button.phalcon-debugbar-metric.is-active .phalcon-debugbar-indicator-icon {
+    stroke: #fff;
+}
+
 #phalcon-debugbar .phalcon-debugbar-request-method {
     color: #a78bfa;
 }

Co-authored-by: Codex <codex@openai.com>
@Alistar84

Alistar84 commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor Author

Hey @niden, thanks again for the really thorough review and for sharing the UI diff — it was very helpful. I’ve pushed the changes in 1367f55.

Here’s the rundown:

  1. History no longer resolves url or response during boot. The endpoint keeps the DI container, resolves the final URL service at request time, and the controller gets its own response.
  2. History now uses its own random phalcon-debugbar-history cookie and never reads, starts, or changes the application session.
  3. Absolute base URIs are reduced to their path, and history.url is validated as an internal absolute path without scheme, host, query, or fragment.
  4. HistoryEndpoint::CONTROLLER_NAMESPACE is public and the docs include the ACL allow example.
  5. Time and Memory now render once as clickable indicators, work with History enabled or disabled, and the request control is capped at 25vw.
  6. The CHANGELOG now uses the correct “Logs” panel name.
  7. The docs explicitly say Time and Memory are enabled by default.
  8. A request is stored only when it already carries the debugbar cookie. The first allowed response sets the cookie but creates no history directory.
  9. Since storage is browser-cookie scoped, session ID regeneration no longer hides earlier entries.
  10. Opening History remembers the active collector, and selecting a stored request reopens it. This works for regular tabs and indicator panels.
  11. History endpoint responses return early from ResponseListener, before collection, headers, injection, or storage.
  12. The panel-type lists and the Time/Memory indicator behavior are corrected in the docs.
  13. I applied the cleanup items as well: removed the dead fallback, simplified controller closures, fixed the docblock, merged duplicate CSS selectors, imported constants, removed the redundant Memory panel declaration and removed suppressed cleanup calls from tests. One tiny exception: I kept metadataFromArray() before metadata() because the repository’s PHP CS Fixer orders static methods before instance methods and changes it back during the quality check.

I also tested the integration in a real Phalcon app: the cookie and stored requests survive logout/login, while anonymous access remains governed by the app ACL.

Final checks are green: 318 PHP tests / 955 assertions, 19 JS tests, 100% coverage, PHPStan, PHPCS and PHP CS Fixer. The commit also keeps the Codex co-author trailer.

Whenever you have a chance, it should be ready for another look. Thanks!

@niden

niden commented Sep 22, 2026

Copy link
Copy Markdown
Member

@Alistar84 Thank you again for this. I will review it more thoroughly later on today. Two things that popped up from a quick scan - we need to update the PR description with all the latest changes (esp. with the cookie changes). Also the changelog needs a couple of pieces to be moved to the Added instead of changed.

I need to test this with Vokuro just to be sure on the visual side but looking really good otherwise.

More a bit later.

@niden

niden commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

@Alistar84 two more issues to address.

  • The history cookie deletes the app's session cookie on the first response (src/DebugBar/History/HistoryCookie.php:55-58, called from src/DebugBar/ResponseListener.php:74)

queue() adds the cookie with $response->setRawHeader('Set-Cookie: …'). Both Phalcon versions send raw headers with replace on: header($header, true) in cphalcon Headers.zep and header($header) in vendor/phalcon/phalcon/src/Http/Response/Headers.php:117. In PHP, replace deletes every earlier header with the same name. session_start() and setcookie() add their Set-Cookie headers during dispatch, which is before sendHeaders(). So on the first allowed response for a new browser, the session cookie (and any other native cookie) is dropped.

The session from that first page is lost. For example, in Vokuro, the first login submit in a new browser fails the CSRF check, and flash messages disappear.

This comes from reading the Phalcon and PHP source. PHP's own ext/session comment warns about this exact case. I could not reproduce it at runtime, because PHP CLI does not record headers. The unit tests only check the raw header string, so they cannot see this.

Options:
i. In queue(), call native setcookie(self::NAME, $id, ['path' => $path, 'httponly' => true, 'samesite' => 'Lax']). It adds the header and never replaces. For tests, inject a callable that defaults to setcookie.
ii. Add the header through a small seam that calls header($line, false).

(Do not use the app's cookies service. It can encrypt cookies and needs a crypt service, which couples the bar to the app's setup.)

  • Cookie attributes (HistoryCookie.php:57)

The cookie has no Secure flag on HTTPS. It also has no Max-Age, so it is a session cookie. History "disappears" when the browser restarts, but the files stay on disk until the TTL ends.

Options:
i. Add Secure when $request->isSecure(), and set Max-Age to ttl_seconds.
ii. Add only Secure, and document that history lasts for one browser session.

@Alistar84

Copy link
Copy Markdown
Contributor Author

Thanks @niden, good catch - and I’m sorry I missed the interaction with Phalcon’s header replacement behavior. My unit tests only inspected the queued raw header, so they did not exercise what happens when the response headers are actually sent.

I agree that the current setRawHeader() approach can replace previously queued Set-Cookie headers.

My preference is option (i): use native setcookie() behind an injectable callable, so it appends the debug bar cookie independently of Phalcon’s response headers and remains testable without coupling the debug bar to the application cookies service.

For the attributes, I would set Secure from RequestInterface::isSecure() and keep it as a browser-session cookie, documenting that the associated files may remain until ttl_seconds after that browser identity expires. This avoids introducing cookie renewal and expiry semantics in this PR.

Would this approach work for you, or would you prefer the cookie expiry to match ttl_seconds?

@niden

niden commented Sep 24, 2026

Copy link
Copy Markdown
Member

@Alistar84 this is your baby. I am fine with either option.

Co-authored-by: Codex <codex@openai.com>
@Alistar84 Alistar84 changed the title Add session-scoped request history browser Add browser-scoped request history browser Sep 24, 2026
Co-authored-by: Codex <codex@openai.com>
@Alistar84
Alistar84 requested a review from niden September 24, 2026 13:57
@niden

niden commented Sep 24, 2026

Copy link
Copy Markdown
Member

@Alistar84 Have a look below

  • Unbounded storage growth from forged browser cookies

(src/DebugBar/History/FilesystemHistory.php:208) - browserDirectory() trusts any cookie matching /^[a-f0-9]{64}$/. The value is never bound to the server, so a client can mint a new identity per request. max_requests caps entries per browser, not the number of browsers, and garbageCollect() only removes entries older than ttl_seconds (default 24h). With the default access.allow_ips => [] (allows any client), this is an unauthenticated disk-fill. You can verify this with two browser sessions.

Options:
i. HMAC-sign the cookie value with a per-install secret and reject unsigned identities in HistoryCookie::id(), so only server-issued identities create storage;
ii. cap the total browser-directory count in garbageCollect() and evict the oldest. The docs already advise access.allow_ips for shared hosts - that mitigates it but does not make the default safe.

  • CommonJS export leaks into host pages (resources/assets/debugbar.js:586) - the module.exports block runs before the typeof document === 'undefined' guard on line 593. A host page that defines a global module with exports (some legacy UMD bundles do) gets it overwritten by an injected debug bar. Moving the export inside the no-document branch keeps the tests working, since the first require() in debugbar.test.js has no document.

I have not verified the above but it might worth a look

  • No X-Content-Type-Options: nosniff (src/DebugBar/Controllers/HistoryController.php:102) - json() sets Cache-Control: no-store, private but not nosniff. Cheap hardening for a response that echoes attacker-influenceable request URIs and headers.

  • history.path writability is never validated (src/DebugBar/History/HistoryOptions.php:55) - validate() checks only "absolute". An unwritable path fails silently forever: save() returns null and nothing surfaces. Checking writability at boot would turn a silent no-op into a clear error.

This is similar to the one we found in round 2 I think.

  • find() leaves empty directories behind (src/DebugBar/History/FilesystemHistory.php:105) - it calls removeExpired() but not removeDirectoryIfEmpty(), unlike get() (line 144) and clear() (line 97).

  • Glob escaping on Windows (src/DebugBar/History/FilesystemHistory.php:294) - matching() escapes \ as a glob metacharacter, but on Windows \ is the path separator, and isAbsolutePath() deliberately accepts C:\... and UNC paths. I could not test this; worth confirming glob still matches there.

  • Hard-coded collector name in the JS (resources/assets/debugbar.js:129) - var request = data.request || {} couples the request indicator to the literal name request, while every other indicator is driven by meta.widgets. A RequestCollector-declared indicator would be consistent with the rest.

They are all pretty small fixes. The first one is the most worrying one. Again, this is a debugbar intended in a local setup but you never know who will decide to push this to production with no guards on it....

Co-authored-by: OpenAI Codex <codex@openai.com>
@Alistar84

Copy link
Copy Markdown
Contributor Author

Thanks @niden, I addressed all the points in 0171cdf.

  • Added history.max_browsers with least-recently-updated eviction. Browser-directory creation is protected by a filesystem lock, and a new identity is refused if eviction fails. I chose a global cap because signing alone would still allow a client to obtain multiple server-issued identities by repeatedly discarding its cookie.
  • Moved the CommonJS export inside the no-document branch.
  • Added X-Content-Type-Options: nosniff to History JSON responses.
  • History now creates and validates the storage directory during boot, failing with a clear error when it is not writable.
  • find() now removes the browser directory when expiration leaves it empty.
  • Windows-style paths are normalized to portable separators before building glob patterns.
  • Request method and URI are now discovered through metadata declared by RequestCollector, without relying on the hard-coded request name.

I also updated the documentation and PR description and added regression tests for forged/rotated identities, concurrent creation, failed eviction, CommonJS host isolation, Windows glob paths, storage validation, empty-directory cleanup, and renamed request collectors.

All CI checks are green, including Code quality, Coverage, and the complete Phalcon 5/6 matrix.

@niden

niden commented Sep 26, 2026

Copy link
Copy Markdown
Member

@Alistar84 Thank you once more for this work. Exemplary. This will make a great addition to this component.

There are a few issues that need attending but those I added them in a different issue for a follow up #32

For now this is merged. Thank you again.

@niden
niden merged commit 4848641 into phalcon:master Sep 26, 2026
12 checks passed
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.

Support for browsing previous requests (request history / multi-request navigation)

2 participants