Skip to content

fix(desktop): recover undecryptable connection catalogs safely - #13656

Open
widingmarcus-cyber wants to merge 1 commit into
pingdotgg:mainfrom
widingmarcus-cyber:codex/8341-recover-undecryptable-catalog
Open

widingmarcus-cyber wants to merge 1 commit into
pingdotgg:mainfrom
widingmarcus-cyber:codex/8341-recover-undecryptable-catalog

Conversation

@widingmarcus-cyber

@widingmarcus-cyber widingmarcus-cyber commented Sep 25, 2026 •

Copy link
Copy Markdown

What Changed

When Electron cannot decrypt the saved connection catalog, preserve the encrypted file under a unique .undecryptable filename before returning an empty catalog. Local environment discovery can then continue, and the user can register remote connections again.

Serialize catalog reads, writes, and clears so recovery cannot rename a concurrent replacement save within the store. If preservation fails, keep reporting an error and leave the original catalog intact. Encryption remains required; other read, decode, and legacy migration failures retain their existing behavior.

Why

Fixes #8341.

On Windows with Alpha 0.0.42, a decrypt-catalog error left the sidebar empty even though the local server responded and the conversation database passed its integrity check. Renaming the connection file aside restored successful catalog reads. The original crash and reason decryption stopped working are unconfirmed.

Preserving the encrypted file before recovery addresses the potential overwrite/data-loss concern raised on #4454. Saved remote connections may need to be registered again; the original encrypted data remains available for manual recovery.

Validation

  • All 10 DesktopConnectionCatalogStore tests pass, including concurrent recovery, exact backup preservation, subsequent encrypted writes, and preservation failure.
  • Both new regression cases fail against unmodified main; the recovery case reproduces the decrypt-catalog failure.
  • Desktop TypeScript check passes.
  • Targeted lint, formatting, and git diff --check pass.
  • A rebuilt packaged desktop application has not been tested. No UI components changed.

Checklist

  • This PR is small and focused
  • I explained what changed and why
  • UI screenshots/video: not applicable; no UI component or animation changes.

Prepared with GPT-6 in the Codex desktop harness.

Summary by CodeRabbit

  • Bug Fixes
    • When a saved connection catalog can’t be decrypted, the app preserves it as a backup and allows a fresh catalog to be saved.
    • Concurrent catalog reads and writes are handled safely. If preserving an unreadable catalog fails, the app reports an error and leaves the original file unchanged.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Sep 25, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This production change alters the persistence and recovery lifecycle of an encrypted connection catalog that can contain credentials and access tokens. Although the implementation is focused and covered by regression tests, its sensitive-data implications warrant human review.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Sep 25, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: c01a61b2-81c6-4b2b-a91d-5dce896db856

📥 Commits

Reviewing files that changed from the base of the PR and between e5a46d6 and 14640ed.

📒 Files selected for processing (2)
  • apps/desktop/src/app/DesktopConnectionCatalogStore.test.ts
  • apps/desktop/src/app/DesktopConnectionCatalogStore.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The catalog store now serializes get, set, and clear. When catalog decryption fails, get preserves the file as an .undecryptable backup and returns no catalog. A failed preservation returns a write error.

Changes

Connection catalog recovery

Layer / File(s) Summary
Serialize catalog operations
apps/desktop/src/app/DesktopConnectionCatalogStore.ts
A one-permit semaphore serializes get, set, and clear.
Preserve undecryptable catalogs
apps/desktop/src/app/DesktopConnectionCatalogStore.ts, apps/desktop/src/app/DesktopConnectionCatalogStore.test.ts
On a decryption error, get renames the catalog to a UUID-suffixed .undecryptable backup and returns None. A rename failure becomes a preserve-undecryptable-catalog write error. Tests cover concurrent reads, backup preservation, and rename failure.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 14640

Catalog recovery preserves the undecryptable file and allows a fresh catalog to be created. No identified issue blocks merging after normal checks.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to 14640

Recovery preserves connections without exposing their contents, but it can leave an encrypted copy of old connection data after the active catalog is cleared. The intended retention and removal behavior merits review.

Retained concerns

  • Medium · security · inferred: After recovery, clearing the catalog removes the canonical file but leaves its encrypted backup. If clear is relied on to remove saved connection credentials, that data remains recoverable by someone who can access and decrypt the backup. Retaining it for manual recovery is intentional; the applicable erasure expectation is not established.
Security review details

Security Blast Radius

  • inferred — The affected data is the saved catalog for one desktop state directory. Its preserved copy can contain encrypted remote-connection credentials; the evidence does not establish broader service or tenant exposure.

Security Findings and Attack Paths

  • inferred — The retention concern requires access to the preserved file and a way to decrypt it; neither capability is demonstrated for a lower-trust actor. No new IPC method or plaintext disclosure is evidenced.

Trust Boundaries and Controls

  • observed — The existing renderer-facing IPC methods still delegate to the desktop store. The store checks encryption availability and decrypts before returning catalog contents; recovery returns none instead of bypassing decryption.

Resilience and Maintainability Implications

  • inferred — Instance-local serialization protects the normal single-store path, but coordination across stores or processes and recovery after interruption are not established. The desktop foundation layer shows one provider, not a filesystem-wide lock.

Hardening Proposals

  • proposed — Define whether clearing saved connections should also dispose of recovery backups, and document or implement the intended manual-recovery and eventual-removal procedure.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: safe recovery of undecryptable desktop connection catalogs.
Description check ✅ Passed The description includes the required change summary, rationale, validation details, and checklist. It also explains that UI screenshots and video are not applicable because no UI components or animat…
Linked Issues check ✅ Passed Issue #8341 requires graceful recovery from an undecryptable connection catalog so the connections UI can continue and a later write can register connections again. DesktopConnectionCatalogStore.get…
Out of Scope Changes check ✅ Passed The changed production code and tests are limited to undecryptable catalog recovery, serialized catalog operations, error propagation for failed preservation, and the related public error type. These …
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M 30-99 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Orphaned safeStorage key makes connection-catalog.json permanently undecryptable, blocking all target listing on Windows

1 participant