Clear proxy config on logout to prevent auto-start without auth#455
Merged
AnthonyRonning merged 1 commit intomasterfrom Mar 5, 2026
Merged
Conversation
- Add stopAndResetProxy() method to proxyService that stops the running proxy and resets saved config to defaults (clearing auto_start and API key) - Call stopAndResetProxy() in all sign-out paths: AccountMenu, DeleteAccountDialog, GuestPaymentWarningDialog, VerificationModal - This prevents the proxy from auto-starting on next app launch when no user is logged in Co-Authored-By: tony@opensecret.cloud <TonyGiorgio@protonmail.com>
Contributor
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Deploying maple with
|
| Latest commit: |
d66c181
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://05eb8bfc.maple-ca8.pages.dev |
| Branch Preview URL: | https://devin-1772686431-clear-proxy.maple-ca8.pages.dev |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis pull request adds proxy service cleanup across multiple sign-out and account deletion flows. A new Changes
Sequence DiagramsequenceDiagram
participant Component as UI Component
participant PS as ProxyService
participant OS as OpenSecret
Component->>Component: User initiates sign-out/delete
Component->>PS: Dynamically import & call stopAndResetProxy()
PS->>PS: Check proxy status & stop if running
PS->>PS: Reset to default configuration
PS-->>Component: Return (with or without error logged)
Component->>OS: Call signOut()
OS-->>Component: Sign-out complete
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/components/AccountMenu.tsx (1)
267-273: Extract this repeated proxy-cleanup block into a shared sign-out helper.The same cleanup snippet is now duplicated across four sign-out/deletion flows. Centralizing it will reduce drift and keep behavior consistent.
Refactor direction
- try { - const { proxyService } = await import("@/services/proxyService"); - await proxyService.stopAndResetProxy(); - } catch (error) { - console.error("Error clearing proxy config:", error); - } + await signOutWithProxyCleanup();// e.g. frontend/src/services/signOutCleanup.ts export async function signOutWithProxyCleanup(): Promise<void> { try { const { proxyService } = await import("@/services/proxyService"); await proxyService.stopAndResetProxy(); } catch (error) { console.error("Error clearing proxy config:", error); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/AccountMenu.tsx` around lines 267 - 273, Extract the repeated proxy-cleanup block into a single helper (e.g. export async function signOutWithProxyCleanup() in a new frontend/src/services/signOutCleanup.ts) that performs the dynamic import of { proxyService } and calls proxyService.stopAndResetProxy() inside the try/catch used today; then replace the duplicated try/catch in AccountMenu.tsx and the other three sign-out/deletion flows with a call to signOutWithProxyCleanup(), keeping existing error handling behavior centralized via that helper and referencing proxyService.stopAndResetProxy to locate the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/services/proxyService.ts`:
- Around line 97-99: The cleanup code in stopAndResetProxy is silently
swallowing errors from stopProxy(), so failures are hidden; change the try/catch
blocks around stopProxy() (and the other similar block) to let errors propagate
to callers instead of ignoring them—either remove the empty catch or rethrow the
caught error (optionally log it via the existing logger before rethrowing) so
callers can observe cleanup failures while sign-out continues; target the
stopAndResetProxy function and the explicit stopProxy() invocation(s) to
implement this change.
---
Nitpick comments:
In `@frontend/src/components/AccountMenu.tsx`:
- Around line 267-273: Extract the repeated proxy-cleanup block into a single
helper (e.g. export async function signOutWithProxyCleanup() in a new
frontend/src/services/signOutCleanup.ts) that performs the dynamic import of {
proxyService } and calls proxyService.stopAndResetProxy() inside the try/catch
used today; then replace the duplicated try/catch in AccountMenu.tsx and the
other three sign-out/deletion flows with a call to signOutWithProxyCleanup(),
keeping existing error handling behavior centralized via that helper and
referencing proxyService.stopAndResetProxy to locate the logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4c8951fa-f18b-4bc5-9f0a-6e2d5fa1930a
📒 Files selected for processing (5)
frontend/src/components/AccountMenu.tsxfrontend/src/components/DeleteAccountDialog.tsxfrontend/src/components/GuestPaymentWarningDialog.tsxfrontend/src/components/VerificationModal.tsxfrontend/src/services/proxyService.ts
Comment on lines
+97
to
+99
| } catch { | ||
| // Proxy may not be running, that's fine | ||
| } |
There was a problem hiding this comment.
Do not silently swallow cleanup failures in stopAndResetProxy.
At Line 97, all status/stop errors are ignored. If stopProxy() fails while running, logout can complete with the proxy still active.
Surface cleanup failures to callers (they already catch) so failures are observable while sign-out still proceeds.
Proposed fix
async stopAndResetProxy(): Promise<void> {
+ let cleanupError: unknown = null;
try {
// Check if proxy is running and stop it
const status = await this.getProxyStatus();
if (status.running) {
await this.stopProxy();
}
- } catch {
- // Proxy may not be running, that's fine
+ } catch (error) {
+ cleanupError ??= error;
+ console.error("Failed to stop proxy during sign-out cleanup:", error);
}
try {
// Save default config to clear auto_start and API key
await this.saveProxySettings({
@@
auto_start: false
});
} catch (error) {
+ cleanupError ??= error;
console.error("Failed to reset proxy config:", error);
}
+
+ if (cleanupError) {
+ throw cleanupError;
+ }
}Also applies to: 111-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/services/proxyService.ts` around lines 97 - 99, The cleanup code
in stopAndResetProxy is silently swallowing errors from stopProxy(), so failures
are hidden; change the try/catch blocks around stopProxy() (and the other
similar block) to let errors propagate to callers instead of ignoring
them—either remove the empty catch or rethrow the caught error (optionally log
it via the existing logger before rethrowing) so callers can observe cleanup
failures while sign-out continues; target the stopAndResetProxy function and the
explicit stopProxy() invocation(s) to implement this change.
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
1 participant
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.
Summary
When a user enables proxy auto-start, logs out, and reopens the app, the proxy would auto-start unnecessarily since the saved config (
~/.config/maple/proxy_config.json) persisted across sessions. This PR resets the proxy config to defaults on every sign-out path, stopping the proxy if running and clearing theauto_startflag and API key.Changes:
stopAndResetProxy()toProxyService— stops the running proxy (if any) and saves a default config to diskstopAndResetProxy()beforeos.signOut()in all 4 sign-out paths:AccountMenu,DeleteAccountDialog,GuestPaymentWarningDialog,VerificationModalReview & Testing Checklist for Human
proxyServicewill invoke Tauri commands that throw on web — both are caught, but check that console noise is acceptable)signOutWithCleanup()utility could reduce this risk — flag if this is preferred before merging.Notes
Summary by CodeRabbit