Skip to content

Implement per-user scoped organization storage - #1886

Open
paustint wants to merge 1 commit into
mainfrom
fix/desktop-per-user-storage
Open

Implement per-user scoped organization storage#1886
paustint wants to merge 1 commit into
mainfrom
fix/desktop-per-user-storage

Conversation

@paustint

Copy link
Copy Markdown
Contributor

Introduce per-user scoped organization storage and migrate legacy organization files to accommodate this change. This enhances data management by associating encryption keys with individual users.

Copilot AI review requested due to automatic review settings July 29, 2026 02:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates Jetstream Desktop’s persisted org storage to be per-user scoped (instead of a single shared orgs.json), and adds a one-time migration path so existing installs can move legacy data into the correct user-scoped file. This aligns the on-disk org data model with per-user encryption keys and avoids cross-account data loss on shared machines.

Changes:

  • Introduce user-scoped org storage files (orgs-<hash>.json) and require userId + encryptionKey to bind org persistence to a specific user.
  • Add legacy migration from the pre-scope shared orgs.json into the signed-in user’s scoped file (with safeStorage-token re-encryption when needed).
  • Update IPC login/auth flows and expand Vitest coverage for multi-account isolation + migration scenarios.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
apps/jetstream-desktop/src/services/persistence.service.ts Implements per-user org file scoping, legacy migration, and updated read/write guards around user-bound storage.
apps/jetstream-desktop/src/services/ipc.service.ts Passes userId alongside the encryption key when initializing org persistence after auth.
apps/jetstream-desktop/src/services/tests/persistence.service.spec.ts Updates and expands tests to cover scoped filenames, multi-account isolation, and legacy file migration.

Comment thread apps/jetstream-desktop/src/services/persistence.service.ts Outdated
Comment thread apps/jetstream-desktop/src/services/persistence.service.ts Outdated
Comment thread apps/jetstream-desktop/src/services/persistence.service.ts Outdated
Copilot AI review requested due to automatic review settings July 29, 2026 02:42
@paustint
paustint force-pushed the fix/desktop-per-user-storage branch from 8a54466 to 4d16a51 Compare July 29, 2026 02:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@paustint
paustint force-pushed the fix/desktop-per-user-storage branch from 4d16a51 to aad5f47 Compare July 29, 2026 15:29
Copilot AI review requested due to automatic review settings July 29, 2026 15:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

apps/jetstream-desktop/src/services/persistence.service.ts:498

  • When quarantining a corrupt per-user orgs file, the backup path is created via rename only. If the file’s permissions were too loose (e.g. restored from a backup or created by an older build), the .corrupt-* copy will retain those mode bits and can remain readable by other local accounts. Consider chmod’ing the backup best-effort to SECURE_FILE_MODE after the rename succeeds.
        renameSync(ORG_FILE_PATH, corruptBackupPath);
        logger.info(`Backed up unreadable orgs file to ${corruptBackupPath}`);

Comment thread apps/jetstream-desktop/src/services/persistence.service.ts Outdated
Copilot AI review requested due to automatic review settings July 30, 2026 02:18
@paustint
paustint force-pushed the fix/desktop-per-user-storage branch from aad5f47 to abe6b1b Compare July 30, 2026 02:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

apps/jetstream-desktop/src/services/persistence.service.ts:150

  • The migration log message claims the legacy orgs file "belongs to a different account" whenever decryption fails. For legacy safeStorage files, a decrypt failure can also mean the file was encrypted on a different machine/VDI session or is corrupted, so this message is misleading for troubleshooting. Consider logging a message that distinguishes portable-vs-safeStorage failure reasons (while still leaving the file untouched).
    } catch (decryptError) {
      logger.info('Legacy orgs file did not decrypt for this user — it belongs to a different account, leaving it untouched', decryptError);
      return;

@paustint
paustint force-pushed the fix/desktop-per-user-storage branch from abe6b1b to eab0c1c Compare July 31, 2026 14:55
Copilot AI review requested due to automatic review settings July 31, 2026 14:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

apps/jetstream-desktop/src/services/persistence.service.ts:88

  • The thrown error for a missing userId is labeled as an "org encryption key" problem, but the actual invalid input is the missing userId. This makes logs/tests harder to interpret when org storage isn’t scoped correctly.
export function bindOrgStorageToUser({ userId, encryptionKey }: { userId: string; encryptionKey: string }): void {
  if (!userId) {
    throw new Error('Invalid org encryption key: a userId is required to resolve the user-scoped org file');
  }

Copilot AI review requested due to automatic review settings July 31, 2026 15:16
@paustint
paustint force-pushed the fix/desktop-per-user-storage branch from eab0c1c to 5544086 Compare July 31, 2026 15:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

apps/jetstream-desktop/src/services/persistence.service.ts:33

  • The comment for LEGACY_SFDC_ORGS_FILE says the legacy orgs.json is "read once per user", but the implementation retries adoption on later cold-cache reads when the scoped file still doesn’t exist (e.g. decrypt failure, transient read error, or file belonging to another account). Updating this comment will avoid misleading future maintainers about the retry behavior.
/**
 * Pre-user-scoping org file, shared by every account that signed in on this machine.
 * Read once per user for migration, never written to again.
 */
const LEGACY_SFDC_ORGS_FILE = join(userData, 'orgs.json');

Comment thread apps/jetstream-desktop/src/services/persistence.service.ts
Copilot AI review requested due to automatic review settings August 1, 2026 22:11
@paustint
paustint force-pushed the fix/desktop-per-user-storage branch from 5544086 to 36c8604 Compare August 1, 2026 22:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment on lines +495 to +508
// safeStorage predates per-user keys entirely and is machine- rather than user-scoped, so
// ownership can't be proven for those files. The first account to sign in claims it, which
// matches the single-shared-file behavior it was originally written under.
orgsJson = isPortable ? decryptOrgsData(rawData, session.key) : safeStorage.decryptString(rawData);
} catch (decryptError) {
logger.info('Legacy orgs file did not decrypt for this user — it belongs to a different account, leaving it untouched', decryptError);
return null;
}

const { jetstreamOrganizations, salesforceOrgs } = OrgsPersistenceSchema.parse(JSON.parse(orgsJson));
return {
jetstreamOrganizations,
salesforceOrgs: isPortable ? salesforceOrgs : salesforceOrgs.map(reEncryptLegacyOrgToken),
};
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants