Cancel revoked remote debugger handshakes - #108
Open
Jim8y wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
RemoteDebugConnectionGeneration.Invalidate() disposes the previous token source, which can cause ObjectDisposedException during in-flight attempt retries and break the intended revocation/cancellation behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens the remote-debugging service against post-revocation completion by tying each connection attempt to a cancellable “generation” and re-checking authorization before persisting trust or publishing/using a connection, with new targeted regression tests for audit item P1-04.
Changes:
- Add
RemoteDebugConnectionGenerationand integrate it intoRemoteDebugServiceto cancel/reject stale connection attempts and revoked commands. - Re-check authorization on the UI thread during session creation and during trusted debugger persistence.
- Add a standalone test project (
tests/p1-04) covering revocation and disposal behaviors; include it in the solution.
File summaries
| File | Description |
|---|---|
| tests/p1-04/TestBoundary.cs | Adds minimal platform/app stubs to run remote-debug service tests without real UI/storage/discovery. |
| tests/p1-04/RevocationTests.csproj | Introduces a dedicated test project that compiles the remote-debug service sources into a standalone harness. |
| tests/p1-04/RevocationTests.cs | Adds tests for generation invalidation canceling handshakes and preventing reactivation of old attempts. |
| tests/p1-04/DisposalTests.cs | Adds tests to ensure disposal drains in-flight connection work and rejects queued connects. |
| OneGateApp/Services/RemoteDebug/RemoteDebugService.cs | Implements generation-based cancellation and additional authorization rechecks; refactors disposal to drain queued work. |
| OneGateApp/Services/RemoteDebug/RemoteDebugConnectionGeneration.cs | Adds a generation + cancellation-token mechanism for revoking connection attempts. |
| OneGateApp.slnx | Adds the new P1-04 test project to the solution. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| @@ -307,11 +342,21 @@ void OnConnectionDisconnected(RemoteDebugConnection sender) | |||
| StateChanged?.Invoke(this, EventArgs.Empty); | |||
Comment on lines
+10
to
+33
| public readonly record struct Attempt(long Version, CancellationToken Token); | ||
|
|
||
| public Attempt Capture() => new(version, cancellation.Token); | ||
|
|
||
| public void Invalidate() | ||
| { | ||
| var previous = cancellation; | ||
| cancellation = new(); | ||
| version++; | ||
| previous.Cancel(); | ||
| previous.Dispose(); | ||
| } | ||
|
|
||
| public void ThrowIfStale(Attempt attempt) | ||
| { | ||
| if (attempt.Version != version || attempt.Token.IsCancellationRequested) | ||
| throw new OperationCanceledException("The remote-debug connection was revoked.", attempt.Token); | ||
| } | ||
|
|
||
| public void Dispose() | ||
| { | ||
| cancellation.Cancel(); | ||
| cancellation.Dispose(); | ||
| } |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
A pending debugger handshake could finish after developer mode was disabled or its debugger was forgotten, then persist trust or publish a live connection after revocation.
Change
This independent PR addresses audit P1-04 only, based on master
623603d. Existing app-link/new-window session navigation is unchanged.Validation
41be6edff6a0d6be86f6ef8ddee2ab04db4cbba3b40056e9c925ebb4410ac31bagainst the first PR commit3fe4e54; both platforms tested this exact follow-up.Screenshots, result JSON and simulator-only fixtures remain outside the repository. Screenshots have not been uploaded to GitHub; hosted CI is not claimed as passing.