Repository navigation
fix(cli): auth session revoke closes the session's open connections #24881
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
only21mil
wants to merge
10
commits into
pingdotgg:main
Choose a base branch
from
only21mil:fix/a174-cli-session-revocation-does-not-invalida
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+412
−44
Open
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
c502be2
fix(cli): auth session revoke closes the running server's open connec…
only21mil afb2e89
fix(server): revoking an already-revoked session still ends its open …
only21mil ab3a235
Merge remote-tracking branch 'github/main' into maria/takeover-pr-24881
maria-rcks 75818d1
test(cli): verify revoke cleanup and preserve current main
maria-rcks 545874c
test(cli): await revocation event delivery
maria-rcks 576d153
fix(cli): merge main into session revocation branch
maria-rcks 0d1dc87
Merge remote-tracking branch 'upstream/main' into fix/a174-cli-sessio…
only21mil 01667fd
fix(cli): session revoke only contacts the server that owns this T3 home
only21mil cf8bcdb
fix(cli): auth session revoke also clears the local session record
only21mil 0da1218
fix(cli): session revoke checks the recorded server process before co…
only21mil File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note
🤖 Opus 5.5 replying on behalf of Victor.
Thanks, you're right: a live PID doesn't show that the recorded origin still belongs to the server that wrote
server-runtime.json. Fixed in 01667fd: the CLI now only contacts the server instance that owns this T3 home.revokeSessioninapps/server/src/cli/auth.tsreads the origin's unauthenticated/.well-known/t3/environmentdescriptor (the endpointt3 pairalready probes). It goes on only whenenvironmentIdmatches this home'senvironment-id. A mismatch, an unreadable descriptor or a timeout means no session is created and noAuthorizationheader is sent. The CLI revokes in the database and prints the existing warning.serverOwnership.tsallows only one. It also needs no per-platformpscall.apps/server/src/cli/pair.test.ts: "sends no credential when the recorded origin serves another T3 home". It recordsprocess.pid(alive) with an origin served by a listener that records every request and returns another home's descriptor. The test asserts the listener got exactly one request,GET /.well-known/t3/environmentwith noAuthorization, the session is revoked locally, and the warning is printed. On the previous head the same test fails: the listener receivesPOST /api/auth/clients/revokewith a bearer. The positive socket-close test (its server now also serves this home's descriptor) and the unreachable-server test are unchanged in what they check.POST /api/auth/clients/revoke authorization=present. After, it received onlyGET /.well-known/t3/environment authorization=absent. Against a realt3 servefor the same home, the revoke goes through the server with no warning.On #23277: still open and still conflicts with this branch in both files, so the landing note in the description stands. I added that the rebase keeps this check.