Harden JWT validation, revocation, and key publication - #3
Conversation
JWT registered date claims are parsed before signature validation. Malformed external values could therefore escape the provider as native PHP errors, while an epoch-zero expiration was incorrectly treated as an absent claim.\n\nTranslate parser failures at the untrusted decode boundary into TokenInvalidException without hiding application-owned encode failures. Treat only null expiration as absent, and document why not-before validation remains active during refresh.\n\nAdd focused regressions for malformed registered dates, string-zero parsing, epoch-zero expiration, and the existing leeway behavior.
JWT previously maintained separate refresh and blacklist lifetimes, ignored cache write failures, and cleared guard state before revocation had settled. Those gaps could allow a refreshable token to outlive its blacklist entry or report a successful logout without durable invalidation.\n\nUse refresh_ttl as the single acceptance and retention lifetime, including expiration leeway and the final minute boundary. Reject missing issued-at claims before infinite refresh, use one clock snapshot for finite retention, honor grace periods without allowing repeated refreshes to extend them, and keep explicit permanent invalidation immediate.\n\nReturn and enforce real storage results through the manager and guard. Preserve guard state and suppress Logout when persistence fails. Remove the destructive unused PSR adapter, retain the tagged-cache and custom-storage extension points, and keep configuration defaults at their owning merged boundary.\n\nThe regressions cover finite and infinite refresh, missing claims, delayed cache reads, leeway, grace, false writes and flushes, zero identifiers, provider replacement, custom storage, and transactional logout.
The certificate command wrote private and public keys directly, accepted RSA sizes rejected by the installed signer, and treated the valid passphrase string zero as empty. A failed or interrupted write could leave incomplete key material with permissive modes.\n\nUse the framework Filesystem owner to create the directory and atomically replace both key files with explicit private and public permissions. Reject RSA keys below 2048 bits before OpenSSL work begins, validate exported public-key contents, and preserve every non-empty passphrase.\n\nDeclare the direct Filesystem dependency and cover supported publication, file modes, invalid RSA sizes, passphrase encryption, overwrite behavior, and the existing EC validation paths.
Bring the canonical JWT guide in line with the corrected runtime behavior. Explain custom signing drivers and storage implementations, the taggable-cache requirement, unified refresh and blacklist retention, grace-aware invalidation, transactional logout failures, and the RSA key-size floor.\n\nKeep the prose application-focused and Laravel-shaped. Narrow the refresh example to token validity failures, describe key configuration accurately, and avoid presenting infrastructure or configuration failures as authentication errors.
Make the package README a thin entry point instead of a second documentation surface. Link to the canonical JWT guide, retain only the public differences developers must account for, and keep the tracked upstream reference last.\n\nRemove implementation detail and duplicated package guidance that would otherwise drift from the framework documentation.
Bring the remaining JWT fixtures in line with the repository's test conventions. Add explicit void returns, remove unused untyped state, and leave each test responsible only for behavior it actually exercises.\n\nThese changes keep the suite strict and readable without adding production code or test-only framework machinery.
Several tests passed multiple method names to shouldNotReceive even though Mockery registers that call as one method expectation. The assertions looked strict but did not prohibit the intended calls.\n\nRegister each prohibited method through a never expectation and remove dead arguments from the single Database prohibition. This makes the existing tests enforce their stated contracts without changing framework behavior or adding recurrence machinery.
Add the reviewed JWT correctness, security, and lifecycle plan with its evidence, rejected designs, regression coverage, performance assessment, and completion criteria.\n\nRecord the final jwt-01 through jwt-15 decisions in the companion ledger, close the JWT checklist, and mark the shared enum-identifier and Macroable dependency revalidations complete. Clear the active routing entry now that implementation, verification, self-review, and independent review have all finished.
…security-lifecycle # Conflicts: # docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe JWT package updates token validation, blacklist lifetimes, storage contracts, logout behavior, configuration, certificate generation, documentation, dependency metadata, regression tests, and audit records. ChangesJWT audit and lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant JwtGuard
participant JwtManager
participant Blacklist
participant Storage
JwtGuard->>JwtManager: Invalidate token during logout
JwtManager->>Blacklist: Add token to blacklist
Blacklist->>Storage: Persist with finite or permanent retention
Storage-->>Blacklist: Return success or failure
Blacklist-->>JwtManager: Return persistence result
JwtManager-->>JwtGuard: Clear state or propagate exception
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 SummaryThe PR hardens JWT temporal validation, revocation persistence and retention, logout settlement, storage contracts, and signing-key publication while updating package configuration and documentation.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/jwt/src/Blacklist.php | Implements unified finite and permanent revocation retention, leeway-aware boundaries, non-sliding repeated revocations, and truthful storage-result propagation. |
| src/jwt/src/JwtManager.php | Enforces issued-at requirements during refresh and turns failed blacklist persistence into an exception. |
| src/jwt/src/JwtGuard.php | Settles logout state and dispatches its event only after revocation succeeds. |
| src/jwt/src/Providers/Lcobucci.php | Normalizes failures parsing untrusted registered-date claims into TokenInvalidException. |
| src/jwt/src/Console/JwtGenerateCertsCommand.php | Enforces minimum RSA strength, preserves string-zero passphrases, and publishes keys atomically per file with explicit modes. |
| src/jwt/src/Contracts/StorageContract.php | Defines truthful boolean persistence and flush results for custom blacklist storage implementations. |
| src/jwt/src/Storage/TaggedCache.php | Returns underlying tagged-cache mutation results without discarding write failures. |
| src/jwt/src/JwtServiceProvider.php | Wires nullable refresh retention, expiration leeway, provider fallbacks, and custom storage resolution. |
| src/jwt/src/Validations/ExpiredClaim.php | Distinguishes an absent expiration claim from epoch zero so zero is correctly rejected as expired. |
| tests/Jwt/BlacklistTest.php | Adds broad regression coverage for revocation boundaries, grace behavior, identifiers, delayed clocks, and persistence failures. |
Reviews (2): Last reviewed commit: "Clarify JWT revocation and key publicati..." | Re-trigger Greptile
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 `@docs/plans/2026-08-08-0426-jwt-correctness-security-and-lifecycle.md`:
- Around line 225-235: Update the JWT key publication flow around the
$files->replace calls to prevent --force from leaving privateKeyPath and
publicKeyPath mismatched when either write fails. Use versioned staging paths
and activate both files only after both replacements succeed, or implement
equivalent rollback/transaction support at the owning Filesystem boundary; then
revise the JWT ledger’s rejected-design and completion claims to reflect the
handled failure mode.
- Around line 159-161: Update Blacklist::addForeverWithGracePeriod() so
grace-protected permanent revocations use an atomic insert/put-if-absent
operation rather than a separate missing() check followed by forever(),
preventing concurrent refreshes from overwriting and extending the grace window.
Preserve the existing grace timestamp and explicit addForever() sentinel
behavior.
In `@src/jwt/README.md`:
- Line 6: Update the “Differences From php-open-source-saver/jwt-auth” heading
in the README to use setext Markdown syntax instead of an ATX heading,
preserving the heading text.
In `@src/jwt/src/Blacklist.php`:
- Around line 85-92: Update StorageContract with an atomic create-if-absent or
compare-and-set operation, then replace the separate storage->get and
storage->forever sequence in Blacklist’s grace-period handling with that
operation using the computed $key and valid_until value. Preserve the behavior
of returning true when an entry already exists, without allowing concurrent
refreshes to overwrite it.
In `@src/jwt/src/Console/JwtGenerateCertsCommand.php`:
- Around line 118-120: Update the key-writing flow around
JwtGenerateCertsCommand to publish the private and public keys as one
recoverable pair: write both contents to new versioned paths first, then
activate both configured key references only after both writes succeed. Preserve
the previous key pair until activation completes, and ensure failures before
activation cannot leave the configured paths pointing to mismatched key
versions.
In `@tests/Jwt/JwtServiceProviderTest.php`:
- Around line 282-300: Update the test setup around the BlacklistContract
resolution in tests/Jwt/JwtServiceProviderTest.php lines 282-300 and 305-320:
bind a mock or shared JwtServiceProviderCustomStorage instance before resolving
BlacklistContract, remove the separate storage resolution, and assert the
minutes state on that same bound instance after blacklist->add().
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c9912e12-9e28-4dee-9c7f-e796c7bac55d
📒 Files selected for processing (39)
docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.mddocs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.mddocs/plans/2026-08-08-0426-jwt-correctness-security-and-lifecycle.mdsrc/boost/docs/jwt.mdsrc/jwt/README.mdsrc/jwt/composer.jsonsrc/jwt/config/jwt.phpsrc/jwt/src/Blacklist.phpsrc/jwt/src/ClaimFactory.phpsrc/jwt/src/Console/JwtGenerateCertsCommand.phpsrc/jwt/src/Contracts/StorageContract.phpsrc/jwt/src/JwtGuard.phpsrc/jwt/src/JwtManager.phpsrc/jwt/src/JwtServiceProvider.phpsrc/jwt/src/Providers/Lcobucci.phpsrc/jwt/src/Storage/PsrCache.phpsrc/jwt/src/Storage/TaggedCache.phpsrc/jwt/src/Validations/ExpiredClaim.phpsrc/jwt/src/Validations/NotBeforeClaim.phptests/Auth/AuthPasswordBrokerTest.phptests/Auth/AuthTokenGuardTest.phptests/Console/CommandMutexTest.phptests/Database/DatabaseTransactionsTest.phptests/HttpServer/ServerTest.phptests/Jwt/BlacklistTest.phptests/Jwt/Console/JwtGenerateCertsCommandTest.phptests/Jwt/JwtConfigTest.phptests/Jwt/JwtGuardEventTest.phptests/Jwt/JwtGuardStaticStateTest.phptests/Jwt/JwtGuardTest.phptests/Jwt/JwtManagerTest.phptests/Jwt/JwtServiceProviderTest.phptests/Jwt/Providers/LcobucciTest.phptests/Jwt/Providers/ProviderTest.phptests/Jwt/Storage/PsrCacheTest.phptests/Jwt/Storage/TaggedCacheTest.phptests/Jwt/Validations/ExpiredClaimTest.phptests/Jwt/Validations/RequiredClaimsTest.phptests/WebSocketServer/ServerHandshakeTest.php
💤 Files with no reviewable changes (2)
- tests/Jwt/Storage/PsrCacheTest.php
- src/jwt/src/Storage/PsrCache.php
| Publish through existing owners: | ||
|
|
||
| ```php | ||
| $files->ensureDirectoryExists($directory); | ||
| $files->replace($privateKeyPath, $privateKey, 0600); | ||
| $files->replace($publicKeyPath, $publicKey, 0644); | ||
| ``` | ||
|
|
||
| The directory's default changes deliberately from permissive `0777` to `0755`. Add `hypervel/filesystem` as a direct, sorted JWT split dependency. The root already contains the package. | ||
|
|
||
| JWT tests own directory creation, generated contents, final modes, algorithm/env output, and command validation. They do not repeat Filesystem's checked-write, temporary-file cleanup, or failure-injection suite. Do not add a cross-file transaction, lock, backup, rollback layer, or JWT-specific writer. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Prevent publication of a mismatched key pair.
Lines 225-235 replace the private and public files in separate operations. Filesystem::replace() makes each file atomic, but it does not make the pair atomic. When --force is used, a successful private-key replacement followed by a failed public-key replacement leaves the configured paths pointing to different key pairs. JWT signing and verification can then fail until manual recovery.
Publish through versioned staging paths and activate only after both files are ready, or add rollback/transaction support at the owning filesystem boundary. Revisit the rejected-design and completion claims in the JWT ledger after this failure mode is handled.
🤖 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 `@docs/plans/2026-08-08-0426-jwt-correctness-security-and-lifecycle.md` around
lines 225 - 235, Update the JWT key publication flow around the $files->replace
calls to prevent --force from leaving privateKeyPath and publicKeyPath
mismatched when either write fails. Use versioned staging paths and activate
both files only after both replacements succeed, or implement equivalent
rollback/transaction support at the owning Filesystem boundary; then revise the
JWT ledger’s rejected-design and completion claims to reflect the handled
failure mode.
There was a problem hiding this comment.
Verified the per-file publication boundary. A failed replacement is loud, leaves the environment file unchanged, and does not affect running workers because their key material is already loaded. Re-running the command repairs the deterministic paths. Versioned activation would require a retention scheme and would not preserve configurations that use literal paths. No transaction machinery was added; the plan and ledger now say explicitly that each key file is published atomically.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
command_file=$(fd -a '^JwtGenerateCertsCommand\.php$' | head -n 1)
plan_file='docs/plans/2026-08-08-0426-jwt-correctness-security-and-lifecycle.md'
ledger_file='docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md'
printf '%s\n' '== Command outline =='
ast-grep outline "$command_file" --items all --view expanded
printf '%s\n' '== Command publication code =='
rg -n -C 12 'ensureDirectoryExists|replace\s*\(|privateKeyPath|publicKeyPath|force' "$command_file"
printf '%s\n' '== Plan statements about publication, transactions, or rollback =='
rg -n -i -C 3 'atomic|transaction|rollback|pair|key publication|replace' "$plan_file"
printf '%s\n' '== Ledger statements about publication, transactions, or rollback =='
rg -n -i -C 3 'atomic|transaction|rollback|pair|key publication|replace' "$ledger_file"Length of output: 201380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
command_file=$(fd -a '^JwtGenerateCertsCommand\.php$' | head -n 1)
printf '%s\n' '== Certificate command: relevant lines =='
rg -n -C 16 '\$files->replace|ensureDirectoryExists|privateKeyPath|publicKeyPath' "$command_file"
printf '%s\n' '== JWT plan: publication section =='
sed -n '209,237p' docs/plans/2026-08-08-0426-jwt-correctness-security-and-lifecycle.md
printf '%s\n' '== JWT ledger: completion and rejected-design statements =='
sed -n '2128,2147p' docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.mdLength of output: 11260
@binaryfire, the finding remains valid.
The current command calls Filesystem::replace() for the private key at Line 119 and then for the public key at Line 120. If the second call fails, the deterministic paths contain keys from different generated pairs.
Env::writeVariables() remaining unchanged does not prevent this condition. New workers, restarted workers, or any process that loads the configured key files after the failed command can fail JWT signing or verification. Re-running the command repairs the state, but it does not prevent the availability failure.
The plan and ledger now accurately describe per-file atomic publication. They do not resolve pair-level atomicity. The rejection of a certificate transaction or rollback layer therefore conflicts with the completion claim that key generation cannot publish partial direct writes.
You are interacting with an AI system.
| $files->ensureDirectoryExists($directory); | ||
| $files->replace($privateKeyPath, $privateKey, 0600); | ||
| $files->replace($publicKeyPath, $publicKey, 0644); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Publish the key pair as one recoverable unit.
Filesystem::replace() is atomic per file, not for both files. If the process fails after Line 119 and before Line 120, the configured paths can contain a new private key and an old public key. New JWT signing or verification can then fail.
Write both keys under new versioned paths first. Update the active key references only after both writes succeed. Keep the prior pair available until activation completes.
🤖 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/jwt/src/Console/JwtGenerateCertsCommand.php` around lines 118 - 120,
Update the key-writing flow around JwtGenerateCertsCommand to publish the
private and public keys as one recoverable pair: write both contents to new
versioned paths first, then activate both configured key references only after
both writes succeed. Preserve the previous key pair until activation completes,
and ensure failures before activation cannot leave the configured paths pointing
to mismatched key versions.
There was a problem hiding this comment.
Verified that Filesystem replacement is atomic per file, not across the pair. The remaining failure window is a loud foreground-command failure before the atomic environment-file update; running workers are unaffected, and the deterministic command can be rerun. Versioned paths would change literal-path behavior and require key retention and cleanup machinery. The implementation remains unchanged, while the audit records now describe per-file atomicity precisely.
There was a problem hiding this comment.
@binaryfire, the audit wording is accurate. However, the finding remains valid.
The stable paths are active before and after the command. A process that starts or reloads after the private-key replacement and before the public-key replacement can read a mismatched key pair. An atomic environment-file update does not remove this interval because the environment values still reference the same paths.
A command failure is visible and rerunnable, but it does not prevent that temporary state. Please retain this as a known limitation or publish both keys through an atomic pair-activation mechanism.
(ᵕ—ᴗ—)🐇
You are interacting with an AI system.
| $config->set('jwt.providers.storage', JwtServiceProviderCustomStorage::class); | ||
| $config->set('jwt.refresh_ttl', 5); | ||
| $config->set('jwt.leeway', 120); | ||
|
|
||
| $this->app->forgetInstance(BlacklistContract::class); | ||
|
|
||
| /** @var Blacklist $blacklist */ | ||
| $blacklist = $this->app->make(BlacklistContract::class); | ||
| $storage = $this->app->make(JwtServiceProviderCustomStorage::class); | ||
| $now = Date::now()->timestamp; | ||
|
|
||
| $this->assertSame(5, $blacklist->getRefreshTTL()); | ||
| $this->assertTrue($blacklist->add([ | ||
| 'exp' => $now + 600, | ||
| 'iat' => $now, | ||
| 'jti' => 'foo', | ||
| ])); | ||
| $this->assertSame(13, $storage->minutes); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 \
'JwtServiceProviderCustomStorage|BlacklistContract|->instance\(|->singleton\(' \
tests/Jwt/JwtServiceProviderTest.php
fd -a 'Container.php' src | while IFS= read -r file; do
rg -n -C 4 'function (make|build|resolve)\b|protected .*instances|public .*instances' "$file"
doneRepository: hypervel/components-backup
Length of output: 11380
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the Blacklist implementation/provider bindings without executing repo code.
fd -a 'Blacklist.php|JwtServiceProvider.php|Storage.php' src tests | sort
for file in $(fd 'Blacklist.php|JwtServiceProvider.php|Storage.php' src tests); do
echo "===== $file ====="
wc -l "$file"
rg -n -C 8 'function __construct|Blacklist|StorageContract|jwt\.providers\.storage|providers|binding|blacklist|singleton\(|bind\(|getRefreshTTL|leeway|refresh_ttl' "$file" || true
done
# Inspect the relevant Container behavior for unregistered class binding.
container_file=$(fd -a 'Container.php' src | head -n1)
if [ -n "${container_file:-}" ]; then
sed -n '1050,1105p;1105,1165p;1165,1225p;1680,1745p' "$container_file"
fi
# Read-only verifier: model the container's class binding with and without a shared instance.
python3 - <<'PY'
class Container:
_instances = {}
def instance(self, abstract, concrete):
self._instances[abstract] = concrete
def forgetInstance(self, abstract):
self._instances.pop(abstract, None)
def make(self, abstract):
if abstract in self._instances:
return self._instances[abstract]
return type(abstract)()
class JwtServiceProviderCustomStorage:
def __init__(self):
self.minutes = None
class ContainerMock(Container):
pass
# Original-ish sequence: configure class-string, forget Blacklist, resolve Blacklist and custom storage separately.
app = ContainerMock()
class CustomStorage:
pass
app.instance('jwt.providers.storage', CustomStorage)
storage1 = app.make('BlacklistContract') # implicit storage resolution for fake container
storage2 = app.make(CustomStorage)
print("without singleton instance storage1 is storage2:", storage1 is storage2)
# Proposed fix: store mock in app instance before resolving BlacklistContrat.
app2 = ContainerMock()
shared = CustomStorage()
app2.instance(CustomStorage, shared)
app2.instance('jwt.providers.storage', CustomStorage)
storage3 = app2.make('BlacklistContract')
storage4 = app2.make(CustomStorage)
print("with shared instance storage3 is storage4:", storage3 is storage4)
PYRepository: hypervel/components-backup
Length of output: 22944
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
class Container:
def __init__(self):
self.instances = {}
def instance(self, abstract, concrete):
self.instances[abstract] = concrete
def forgetInstance(self, abstract):
self.instances.pop(abstract, None)
def isShared(self, abstract): # matches relevant path for auto-singleton
return abstract in self.instances
def make(self, abstract):
if abstract in self.instances:
return self.instances[abstract]
class_name = 'CustomStorage'
autoSingletons_key = class_name
if not self.isShared(abstract) and not self.isShared(autoSingletons_key):
self.autoSingletons = {autoSingletons_key: type('CustomStorage', (), {'minutes': None})()}
return self.autoSingletons[autoSingletons_key]
return type(abstract)()
autoSingletons = {}
def run(name, setup):
app = Container()
setup(app)
blacklist = app.make('BlacklistContract')
storage = app.make('CustomStorage')
print(name, 'same_instance=', blacklist is storage)
run('without pre-bound custom storage', lambda app: None)
run('with pre-bound custom storage instance', lambda app: app.instance('CustomStorage', type('CustomStorage', (), {'minutes': None})()))
PY
# Inspect Blacklist to ensure the property is not used by these tests for object identity.
sed -n '230,335p' tests/Jwt/JwtServiceProviderTest.phpRepository: hypervel/components-backup
Length of output: 4430
Make this assertion target the storage bound by BlacklistContract.
BlacklistContract constructs a custom storage via $app->make($storageClass), while the second $app->make(JwtServiceProviderCustomStorage::class) resolves separately. Bind a mock instance before resolving BlacklistContract, then assert the injected state from that same object.
📍 Affects 1 file
tests/Jwt/JwtServiceProviderTest.php#L282-L300(this comment)tests/Jwt/JwtServiceProviderTest.php#L305-L320
🤖 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 `@tests/Jwt/JwtServiceProviderTest.php` around lines 282 - 300, Update the test
setup around the BlacklistContract resolution in
tests/Jwt/JwtServiceProviderTest.php lines 282-300 and 305-320: bind a mock or
shared JwtServiceProviderCustomStorage instance before resolving
BlacklistContract, remove the separate storage resolution, and assert the
minutes state on that same bound instance after blacklist->add().
There was a problem hiding this comment.
No change here. Hypervel auto-singletons unbound concrete classes, so the custom storage resolved inside BlacklistContract and the later make call are the same object. The focused tests pass independently and their mutated state already proves that identity. Prebinding an instance would replace the real production resolution path with a weaker fixture.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
container_files=$(fd -a '^Container\.php$' src || true)
provider_files=$(fd -a '^JwtServiceProvider\.php$' src || true)
printf '%s\n' '=== Container candidates ==='
printf '%s\n' "$container_files"
while IFS= read -r file; do
[ -n "$file" ] || continue
echo "=== $file ==="
rg -n -C 8 'autoSingleton|function (make|resolve|build|isShared|forgetInstance)\b|instances' "$file" || true
done <<< "$container_files"
printf '%s\n' '=== JWT service provider candidates ==='
printf '%s\n' "$provider_files"
while IFS= read -r file; do
[ -n "$file" ] || continue
echo "=== $file ==="
rg -n -C 8 'BlacklistContract|StorageContract|providers\.storage|make\(' "$file" || true
done <<< "$provider_files"Length of output: 28606
@binaryfire, the repository output confirms your explanation.
JwtServiceProvider resolves custom storage through $app->make($storageClass). Container::resolve() then caches that unbound concrete class in $autoSingletons and returns the cached object for the later make() call.
The test observes the same JwtServiceProviderCustomStorage instance. The withdrawal remains correct.
✏️ Learnings added
Learnt from: binaryfire
Repo: hypervel/components-backup PR: 3
File: tests/Jwt/JwtServiceProviderTest.php:282-300
Timestamp: 2026-08-08T16:23:53.121Z
Learning: In Hypervel, `src/container/src/Container.php` auto-singletons unbound concrete classes in `$autoSingletons`. Repeated `make()` calls for the same unbound class return that cached instance. In `src/jwt/src/JwtServiceProvider.php`, custom JWT blacklist storage is resolved with `$app->make($storageClass)`, so a later test `make()` call for the same custom storage class observes the instance injected into `BlacklistContract`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
You are interacting with an AI system.
The existing blacklist read prevents repeated revocations from restarting a configured grace period. It does not provide cross-store atomic insertion for first writes that were already overlapping. Document the bounded scheduling tradeoff and the safety properties that make additional synchronization unnecessary. Clarify that certificate publication is atomic per generated file rather than across the pair. Add the missing method documentation to the public storage extension contract so its finite, permanent, read, removal, and flush operations are explicit.
|
@coderabbitai Follow-up is complete in 2b5877f. The blacklist guard now documents its real contract: repeated revocations do not restart grace, while rare first writes that were already overlapping may produce a bounded cache-latency shift. Zero grace remains immediate, and no cross-store synchronization was added. The audit records now distinguish per-file atomic certificate replacement from pair-level atomicity. A failed foreground command remains loud, leaves the environment file untouched, and is recoverable by rerunning the deterministic command. Versioned activation and retention machinery was intentionally not added. The README heading and custom-storage test were left unchanged because they already follow repository Markdown and container-resolution conventions. The generic docstring threshold was not applied to tests or closures; the public StorageContract methods now carry the concise documentation required by this repository. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git show --no-ext-diff --format=fuller --stat 2b5877f62
printf '\n--- changed paths ---\n'
git diff-tree --no-commit-id --name-status -r 2b5877f62
printf '\n--- focused diff ---\n'
git show --no-ext-diff --format= --find-renames 2b5877f62 -- \
src/jwt/src/Blacklist.php \
src/jwt/src/Contracts/StorageContract.php \
src/jwt/README.md \
tests/Jwt/JwtServiceProviderTest.php \
docs/plansLength of output: 15028
The audit records now state that key publication is atomic per generated key file. The reported README and custom-storage test decisions are accepted. No further follow-up is needed. ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
1 participant
Summary
This change hardens JWT parsing, refresh, revocation, storage, and certificate publication. It fixes cases where malformed date claims could escape the JWT exception boundary, refreshable tokens could outlive their blacklist entries, and cache write failures could be reported as successful invalidation.
The implementation keeps the existing Laravel-shaped manager, guard, facade, and extension APIs. It removes unsafe Hypervel-only surfaces instead of preserving them through compatibility code.
What changed
TokenInvalidExceptionat the untrusted decode boundary.refresh_ttlas the single refresh and blacklist retention lifetime.StorageContract.0.Application impact
Custom
StorageContractimplementations must return truthful booleans fromadd,forever, andflush. Applications should userefresh_ttlfor both refresh and revocation retention; the removedblacklist_refresh_ttlsetting no longer creates a second lifetime. The shipped blacklist storage requires a taggable cache store, while applications using another store may provide their ownStorageContractimplementation.Supported Laravel auth, guard, manager, facade, and named-argument APIs remain unchanged.
Performance
The request path gains no retry, lock, registry, service resolution, or additional network round trip. Terminal tokens skip cache I/O. Finite lifetime calculation is bounded arithmetic using one clock snapshot. Automatic permanent retention performs one existing-entry read so concurrent refreshes cannot restart the grace period; explicit permanent invalidation remains write-only.
Certificate publication is command-only work and uses the existing Filesystem primitive rather than package-owned synchronization or rollback machinery.
Validation
The complete JWT suite and affected Auth, Console, Database, HTTP Server, and WebSocket Server tests pass. Split-package metadata validation, facade linting, targeted PHPStan, stale-reference scans, and
git diff --checkpass. The repositorycomposer fixgate also passes, including formatting, static analysis, the parallel suite, Testbench package mode, and dogfood resolution.Summary by CodeRabbit
file://key references.