Add request-aware runtime configuration hooks#425
Conversation
Add concise agent guidance for three recurring Hypervel conventions.\n\nRecord src/boost/docs as the documentation location, prefer jsonb() for JSON columns so PostgreSQL receives jsonb while other supported databases compile to compatible storage, and require optional framework events to be guarded with hasListeners() so hot paths skip event construction and dispatch when nobody is listening.
Replace Hypervel's custom Sanctum statefulDomains() override point with a boot-time resolveStatefulDomainsUsing() callback that receives the current request. This mirrors the request-aware resolver pattern used by CORS while keeping the resolver itself worker-lifetime static state. The middleware now resolves stateful domains through the callback when one is registered, falls back to sanctum.stateful_domains otherwise, and normalizes the result to non-empty strings. The old subclass fixture test was replaced with coverage for callback precedence, request injection, and flushState() clearing back to config behavior. Register the new Sanctum middleware flushState() with the global PHPUnit after-each cleanup subscriber so resolver state cannot leak between tests. Also clarify Testbench Foundation Application's flushState() comment to make clear it is not the global framework static-state cleanup list. Update the Sanctum docs to show registering the resolver from a service provider instead of replacing the middleware class, with wording that presents per-tenant/per-host rules only as examples of application-specific request-derived domains. Verified with: composer test -- tests/Sanctum/EnsureFrontendRequestsAreStatefulTest.php; composer test -- tests/Sanctum; composer test -- tests/Testbench/Foundation/ApplicationTest.php tests/Testbench/CreatesApplicationTest.php tests/Testbench/TestCaseTest.php tests/Testbench/Foundation/Bootstrap/CreateVendorSymlinkTest.php; composer analyse; composer lint.
Drop UrlGenerator::flushRequestState() now that URL-owned coroutine context is cleaned by the coroutine lifecycle and the test subscriber's global CoroutineContext::flush() call. UrlGenerator::flushState() now only clears the class's macro state. This keeps static-state cleanup focused on actual class-level state instead of duplicating broader context cleanup. The URL facade docblock no longer advertises flushRequestState(), and the routing static-state coverage now treats UrlGenerator like the other Macroable routing classes instead of asserting request-context cleanup through flushState(). Verification: composer fix reached the parallel test suite after passing formatting and static analysis, but hit a Telescope prune sequence collision. A clean follow-up composer test:parallel run passed: 21435 tests, 60722 assertions, 643 skipped.
Remove the EntryModel factory's random sequence default so tests use the database-assigned bigIncrements value from the telescope_entries schema. The previous random_int(1, 10000) assignment manually populated a unique auto-increment column and could collide whenever a test created multiple Telescope entries. That was the source of the intermittent PruneCommandTest unique constraint failure. Refresh the entry model in RouteTest before asserting its sequence because EntryModel uses uuid as a non-incrementing primary key, so Eloquent does not hydrate the database-assigned sequence onto the model after create(). Verification: ./vendor/bin/phpunit tests/Telescope/Http/RouteTest.php; composer test:parallel.
Store URL::useAssetOrigin() overrides in coroutine context instead of mutating the singleton URL generator's configured asset root. This keeps ASSET_URL / app.asset_url as the app-wide asset origin while allowing request-specific asset origins without leaking across concurrent Swoole requests or later requests handled by the same worker. Update asset URL generation to prefer the request-scoped forced asset root, then the configured asset root, then the current request root. Passing null to useAssetOrigin() now clears only the request-scoped override and falls back to the configured/default origin. Document the split between app-wide ASSET_URL configuration and per-request URL::useAssetOrigin() overrides, and add coroutine isolation coverage for both asset origins and normal URL origins. Reviewed-by: Claude via Codesonic. Verification: ./vendor/bin/phpunit tests/Routing/RoutingUrlGeneratorTest.php; composer test:parallel.
Rename the Hypervel-owned passkey relying party ID and allowed origins callbacks to resolveRelyingPartyIdUsing and resolveAllowedOriginsUsing so they match the framework convention for config-derived resolver hooks. Update the internal callback property names, all Passkeys and Fortify tests, and the Boost Fortify documentation examples and worker-state list to use the new API names. Document the naming rule in AGENTS.md, including the intentional redirectUsing exception for Laravel-established redirect hook conventions. Validated with focused Passkeys/Fortify tests and composer fix, including PHP CS Fixer, PHPStan, the full parallel suite, package checks, and dogfood testbench tests. Reviewed and signed off by Claude.
Add TrustHosts::resolveHostsUsing() so applications can resolve trusted host patterns from the current request during middleware execution while preserving the existing hosts(): array override point and TrustHosts::at() boot configuration API. Normalize empty trusted host pattern lists to a named never-matching sentinel so enabled TrustHosts fails closed instead of disabling validation. This covers both the new resolver path and existing dynamic at() callbacks that resolve to no hosts. Document the resolver contract in the requests docs: the resolver is the complete authority for the request, must return every trusted pattern, and returning an empty array rejects the request. Also record the fail-closed behavior as a Laravel difference in the HTTP package README. Add integration coverage for verified request-aware host patterns, unrecognized host rejection, empty legacy closures failing closed, missing app.url configuration failing closed, and resolver cleanup through flushState. Validated with focused TrustHosts tests and composer fix, including PHP CS Fixer, PHPStan, the full parallel suite, package checks, and dogfood testbench tests. Reviewed and signed off by Claude.
Remove the dead forced-root state left behind after Hypervel moved URL origin overrides to request-scoped URL::useOrigin() context. Wayfinder now intentionally preserves app.url base paths and explicit route domains, but no longer pretends to support Laravel's global forced-root generation behavior. Replace Wayfinder's reflection of UrlGenerator internals with a getForcedScheme() accessor so the remaining forced-scheme behavior for explicit domain routes stays supported without private-property reflection. Document forceScheme() and forceHttps() as boot-only because they mutate worker-shared URL generator state. Record the intentional Laravel difference in the Wayfinder README and add an end-to-end generation test proving forced HTTPS still applies to explicit domain routes. Verified with: pnpm test src/wayfinder/tests/AppUrlRootResolution.test.ts; pnpm test; composer test tests/Wayfinder; composer lint; composer analyse.
|
Warning Review limit reached
Next review available in: 20 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR introduces request-aware and boot-time resolver callback APIs across TrustHosts, Passkeys, and Sanctum middleware (e.g., ChangesResolver Callback Migration and Coroutine-Scoped URL Origins
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant TrustHosts
participant HostsResolver
Client->>TrustHosts: handle(request)
TrustHosts->>TrustHosts: resolveTrustedHostPatterns(request)
alt hostsResolver set
TrustHosts->>HostsResolver: call(request)
HostsResolver-->>TrustHosts: host patterns
else no resolver
TrustHosts->>TrustHosts: hosts()
end
alt patterns empty
TrustHosts->>TrustHosts: use REJECT_ALL_HOST_PATTERN
end
TrustHosts-->>Client: allow or SuspiciousOperationException
sequenceDiagram
participant Request
participant EnsureFrontendRequestsAreStateful
participant Config
Request->>EnsureFrontendRequestsAreStateful: fromFrontend(request)
EnsureFrontendRequestsAreStateful->>EnsureFrontendRequestsAreStateful: resolveStatefulDomains(request)
alt resolver registered
EnsureFrontendRequestsAreStateful->>EnsureFrontendRequestsAreStateful: call resolver(request)
else no resolver
EnsureFrontendRequestsAreStateful->>Config: get sanctum.stateful_domains
end
EnsureFrontendRequestsAreStateful-->>Request: stateful/not-stateful decision
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR moves several extension points from process-global or static state into request-aware APIs, making them safe for Hypervel's long-lived Swoole workers where multiple requests run concurrently. The default behavior is unchanged unless an application explicitly opts into a resolver.
Confidence Score: 5/5Safe to merge. The fail-closed defaults, coroutine isolation, and state cleanup are all correctly implemented and well-tested. All new resolver hooks default to the existing static configuration, so no behaviour change unless explicitly opted into. Fail-closed patterns are correctly applied and covered by integration tests. Asset origin override is moved to CoroutineContext with the right precedence order. Boot-only warnings are consistently applied, and every new flushState() entry is registered in AfterEachTestSubscriber. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "fix(http): harden request-aware resolver..." | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/boost/docs/requests.md (1)
927-945: 🧹 Nitpick | 🔵 TrivialConsider noting caching guidance for per-request resolver lookups.
The example queries the database inside the resolver, which runs on every trusted-host check (per
resolveTrustedHostPatterns()in TrustHosts.php). Worth a brief note recommending callers cache verified domains (e.g., in-process cache/Redis) to avoid a DB round-trip on every request.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/boost/docs/requests.md` around lines 927 - 945, The TrustHosts::resolveHostsUsing example in the request docs currently shows a per-request database lookup inside the resolver, which can cause a DB hit on every trusted-host check. Update the guidance near the TrustHosts and resolveTrustedHostPatterns usage to note that callers should cache verified domain lookups (for example with an in-process cache or Redis) before resolving host patterns, and mention that the resolver should read from cache rather than querying VerifiedDomain directly on every request.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/sanctum/src/Http/Middleware/EnsureFrontendRequestsAreStateful.php`:
- Around line 100-119: The registered-resolver branch in resolveStatefulDomains
currently assumes the callback always returns an array of strings, unlike the
config fallback which validates the shape first. Update the
static::$statefulDomainsResolver path to defensively check that the resolver
result is an array and filter out non-string or empty entries before calling
array_filter, matching the hardening already used for
config('sanctum.stateful_domains', []).
In `@tests/Integration/Http/Middleware/TrustHostsTest.php`:
- Around line 66-134: Add explicit : void return types to each newly added test
method in TrustHostsTest, including
testRequestAwareResolverUsesVerifiedHostPatterns,
testRequestAwareResolverRejectsUnrecognizedHosts,
testEmptyLegacyHostClosureFailsClosed,
testMissingTrustedHostConfigurationFailsClosed, and
testFlushStateClearsRequestAwareResolver, so they match the PHPUnit test method
conventions used elsewhere in the class.
---
Nitpick comments:
In `@src/boost/docs/requests.md`:
- Around line 927-945: The TrustHosts::resolveHostsUsing example in the request
docs currently shows a per-request database lookup inside the resolver, which
can cause a DB hit on every trusted-host check. Update the guidance near the
TrustHosts and resolveTrustedHostPatterns usage to note that callers should
cache verified domain lookups (for example with an in-process cache or Redis)
before resolving host patterns, and mention that the resolver should read from
cache rather than querying VerifiedDomain directly on every request.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 498dc312-3e3d-4da0-a796-a284a77ace9b
⛔ Files ignored due to path filters (9)
_archive/docs/0.3/.vuepress/public/favicon.icois excluded by!**/*.ico_archive/docs/0.3/.vuepress/public/home.pngis excluded by!**/*.png_archive/docs/0.3/.vuepress/public/icon.pngis excluded by!**/*.png_archive/docs/0.3/.vuepress/public/icon.svgis excluded by!**/*.svg_archive/docs/0.3/.vuepress/public/images/prompts-example.pngis excluded by!**/*.png_archive/docs/0.3/.vuepress/public/images/whoops.pngis excluded by!**/*.png_archive/docs/0.3/.vuepress/public/logo.pngis excluded by!**/*.png_archive/docs/0.3/.vuepress/public/logo.svgis excluded by!**/*.svg_archive/docs/0.3/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (113)
AGENTS.md_archive/README.md_archive/docs/0.3/.vuepress/config.js_archive/docs/0.3/.vuepress/sidebar.js_archive/docs/0.3/.vuepress/styles/palette.scss_archive/docs/0.3/README.md_archive/docs/0.3/docs/artisan.md_archive/docs/0.3/docs/authentication.md_archive/docs/0.3/docs/authorization.md_archive/docs/0.3/docs/blade.md_archive/docs/0.3/docs/broadcasting.md_archive/docs/0.3/docs/cache.md_archive/docs/0.3/docs/collections.md_archive/docs/0.3/docs/configuration.md_archive/docs/0.3/docs/console-tests.md_archive/docs/0.3/docs/container.md_archive/docs/0.3/docs/context.md_archive/docs/0.3/docs/contracts.md_archive/docs/0.3/docs/contributions.md_archive/docs/0.3/docs/controllers.md_archive/docs/0.3/docs/coroutine.md_archive/docs/0.3/docs/csrf.md_archive/docs/0.3/docs/data-objects.md_archive/docs/0.3/docs/database-testing.md_archive/docs/0.3/docs/database.md_archive/docs/0.3/docs/deployment.md_archive/docs/0.3/docs/eloquent-collections.md_archive/docs/0.3/docs/eloquent-factories.md_archive/docs/0.3/docs/eloquent-mutators.md_archive/docs/0.3/docs/eloquent-relationships.md_archive/docs/0.3/docs/eloquent-resources.md_archive/docs/0.3/docs/eloquent-serialization.md_archive/docs/0.3/docs/eloquent.md_archive/docs/0.3/docs/encryption.md_archive/docs/0.3/docs/errors.md_archive/docs/0.3/docs/events.md_archive/docs/0.3/docs/facades.md_archive/docs/0.3/docs/filesystem.md_archive/docs/0.3/docs/hashing.md_archive/docs/0.3/docs/helpers.md_archive/docs/0.3/docs/horizon.md_archive/docs/0.3/docs/http-client.md_archive/docs/0.3/docs/http-tests.md_archive/docs/0.3/docs/installation.md_archive/docs/0.3/docs/introduction.md_archive/docs/0.3/docs/lifecycle.md_archive/docs/0.3/docs/localization.md_archive/docs/0.3/docs/logging.md_archive/docs/0.3/docs/mail.md_archive/docs/0.3/docs/middleware.md_archive/docs/0.3/docs/migrations.md_archive/docs/0.3/docs/mocking.md_archive/docs/0.3/docs/nested-set.md_archive/docs/0.3/docs/notifications.md_archive/docs/0.3/docs/packages-porting.md_archive/docs/0.3/docs/packages.md_archive/docs/0.3/docs/pagination.md_archive/docs/0.3/docs/permission.md_archive/docs/0.3/docs/processes.md_archive/docs/0.3/docs/prompts.md_archive/docs/0.3/docs/providers.md_archive/docs/0.3/docs/queries.md_archive/docs/0.3/docs/queues.md_archive/docs/0.3/docs/rate-limiting.md_archive/docs/0.3/docs/redis.md_archive/docs/0.3/docs/requests.md_archive/docs/0.3/docs/responses.md_archive/docs/0.3/docs/routing.md_archive/docs/0.3/docs/sanctum.md_archive/docs/0.3/docs/scheduling.md_archive/docs/0.3/docs/seeding.md_archive/docs/0.3/docs/session.md_archive/docs/0.3/docs/socialite.md_archive/docs/0.3/docs/strings.md_archive/docs/0.3/docs/structure.md_archive/docs/0.3/docs/telescope.md_archive/docs/0.3/docs/testbench.md_archive/docs/0.3/docs/testing.md_archive/docs/0.3/docs/urls.md_archive/docs/0.3/docs/validation.md_archive/docs/0.3/docs/views.md_archive/docs/0.3/package.json_archive/docs/0.4/permission.mdsrc/boost/docs/fortify.mdsrc/boost/docs/requests.mdsrc/boost/docs/sanctum.mdsrc/boost/docs/urls.mdsrc/http/README.mdsrc/http/src/Middleware/TrustHosts.phpsrc/passkeys/src/Passkeys.phpsrc/routing/src/UrlGenerator.phpsrc/sanctum/src/Http/Middleware/EnsureFrontendRequestsAreStateful.phpsrc/support/src/Facades/URL.phpsrc/telescope/database/factories/EntryModelFactory.phpsrc/testbench/src/Foundation/Application.phpsrc/testing/src/PHPUnit/AfterEachTestSubscriber.phpsrc/wayfinder/README.mdsrc/wayfinder/src/GenerateCommand.phpsrc/wayfinder/src/Route.phpsrc/wayfinder/tests/AppUrlRootResolution.test.tssrc/wayfinder/tests/generate.phptests/Fortify/PasskeyTest.phptests/Integration/Http/Middleware/TrustHostsTest.phptests/Passkeys/CoroutineIsolationTest.phptests/Passkeys/Feature/Actions/GenerateRegistrationOptionsTest.phptests/Passkeys/Feature/Actions/GenerateVerificationOptionsTest.phptests/Passkeys/Feature/PasskeysTest.phptests/Passkeys/Feature/WebAuthnTest.phptests/Passkeys/PasskeysStaticStateTest.phptests/Routing/RoutingStaticStateTest.phptests/Routing/RoutingUrlGeneratorTest.phptests/Sanctum/EnsureFrontendRequestsAreStatefulTest.phptests/Telescope/Http/RouteTest.php
💤 Files with no reviewable changes (2)
- src/support/src/Facades/URL.php
- src/telescope/database/factories/EntryModelFactory.php
Normalize Sanctum stateful-domain resolver output through the same filtering path as static configuration so malformed resolver values cannot throw while checking a request. Non-array resolver results now degrade to an empty stateful-domain list, matching the existing config fallback behavior. Make TrustHosts treat non-array request-aware resolver results as an empty trusted-host list. This preserves the middleware's fail-closed behavior by flowing through the reject-all sentinel instead of surfacing a raw TypeError. Add focused regression coverage for malformed Sanctum resolver output and fail-closed TrustHosts resolver output. Also align TrustHostsTest methods with the repository's : void test method convention. Document that request-aware trusted-host resolvers run for each request and should resolve against cached verified-host data rather than querying the database every time. Verified with: composer test tests/Sanctum/EnsureFrontendRequestsAreStatefulTest.php; composer test tests/Integration/Http/Middleware/TrustHostsTest.php; composer lint; composer analyse.
|
@greptileai @coderabbitai thanks for the review. I went through the findings against the branch and pushed What changed:
Validation run after the changes:
I also replied on the individual inline threads with the specific resolution for each one. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
git log --oneline -5 76c836875 2>/dev/null || echo "commit not found locally"
git show --stat 76c836875 2>/dev/null | head -50Length of output: 2319
This all looks solid and directly addresses the fail-closed/type-safety concerns raised earlier. I'll check the individual inline thread replies for the specific resolutions as well. [resolve] |
|
The fixes look solid. Here's the summary: Sanctum ( TrustHosts — Test hygiene — all One small observation: Everything looks correct. The hardening is consistent across both components and the test coverage directly targets the failure modes that were raised. No further changes needed on my end. |
Context
Laravel applications can often get away with changing configuration or singleton state while a request is running. A request has a short lifecycle, and that makes mutation a convenient extension point for things like trusted hosts, stateful domains, URL origins, or package-specific runtime options.
Hypervel has a different constraint. The worker outlives the request, and multiple requests can be active in the same worker at the same time. Mutating global configuration or shared singleton state during a request is therefore not just a style issue; it can leak behavior into unrelated requests.
This PR moves several of those extension points to request-aware APIs. The goal is to preserve the ergonomics of configuring request-derived behavior while making the storage model explicit and safe for Hypervel's runtime.
What changed
Sanctum
Sanctum's stateful-domain handling now supports a callback-based resolver. This replaces the previous subclass/override shape with an explicit API that can be registered from a service provider.
The resolver is evaluated at request time and takes precedence over the static
sanctum.statefulconfiguration. Static configuration remains the default behavior when no resolver is registered.URL generation
Asset origin overrides are now scoped to the current request/coroutine instead of being stored as worker-shared URL generator state.
This keeps
URL::useAssetOrigin()useful for request-specific asset hosts without allowing one request's asset origin to bleed into another request. The existing public signature stays the same.The URL generator also no longer exposes a request-state flush helper. Request context is already destroyed centrally by the test/runtime context cleanup path, so a bespoke URL-only flush method added unnecessary API surface.
Trusted hosts
TrustHostsnow supports a request-aware resolver for trusted host patterns.This gives applications a way to decide accepted hosts from the current request without mutating global middleware configuration. The resolver accepts multiple host patterns. If it resolves to an empty set, the middleware fails closed rather than silently trusting everything.
TrustProxiesis intentionally unchanged. Proxy trust is infrastructure-level configuration and should remain fixed for the process.Passkeys
The custom WebAuthn resolver APIs were renamed to match the framework's
resolve...Using(...)convention.This is mostly an API consistency change, but it matters because these callbacks are part of the same request-aware configuration surface. Callers and docs were updated together.
Wayfinder
Hypervel Wayfinder no longer pretends to support Laravel's global forced-root generation behavior.
Laravel can read
URL::forceRootUrl()from a globally-mutated URL generator during code generation. Hypervel's equivalent runtime API isURL::useOrigin(), which is request-scoped by design. Reading that from a build-time command would be the wrong model.Wayfinder now keeps the behavior that fits Hypervel:
app.urlis still preserved for subdirectory deployments;The intentional Laravel difference is documented in the Wayfinder README.
Supporting cleanup
This branch also includes a small amount of supporting cleanup that came out of the same audit:
Why this shape
The common pattern in this PR is: static configuration remains static, request-specific behavior moves behind an explicit resolver or coroutine-scoped value.
That keeps the Laravel-style configuration model where it is safe, and avoids carrying over the parts that depend on a per-request PHP process lifecycle.
This also makes the distinction visible in code. A callback named
resolve...Using(...)or a method documented as request-scoped communicates a very different contract than changingconfig()or a singleton property mid-request.Compatibility
Defaults are unchanged unless an application opts into one of the new resolver hooks.
The main intentional behavior difference is Wayfinder forced-root generation. That behavior was already inert in Hypervel after URL origins became request-scoped. This PR removes the dead plumbing and documents the Hypervel behavior instead of adding a new build-time root configuration surface.
Validation
This branch was reviewed with Claude during development and the final Wayfinder cleanup was signed off after review.
Checks run across the branch included:
composer fixcomposer test:parallelcomposer test tests/Wayfindercomposer lintcomposer analysepnpm testSummary by CodeRabbit
New Features
Bug Fixes
Documentation