Implement per-user scoped organization storage

[paustint]

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]

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.

[Copilot]

Pull request overview

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

[Copilot]

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}`);

[Copilot]

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;

[Copilot]

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]

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');

[Copilot]

Pull request overview

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

[paustint]

Went through Copilot's suppressed (low-confidence) comments — one was valid:

• apps/jetstream-desktop/src/services/persistence.service.ts:500 — fixed, the migration log claimed "it belongs to a different account" on every decrypt failure, which is only true for the portable format. safeStorage failures are just as likely to be a profile that moved machines or a rotated DPAPI key, so the message now distinguishes the two.

The rest didn't apply: the .corrupt-* backup (:498) is a rename of a scoped file that only writeOrgsFile ever creates, always at 0600, so it already inherits owner-only mode; the bindOrgStorageToUser error message (:88) and the LEGACY_SFDC_ORGS_FILE comment (:33) were both already corrected in later commits.

[Copilot]

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:510

• If the legacy shared orgs file exists but cannot be decrypted (wrong per-user key, safeStorage DPAPI change, or corruption), the code intentionally leaves it in place — but it may also retain overly-permissive mode bits forever because nothing will ever rewrite it. Consider best-effort chmod’ing the legacy file to 0600 on this path to prevent other local accounts from reading the encrypted token material, without changing the contents.

          ? 'Legacy orgs file did not decrypt for this user — it belongs to a different account, leaving it untouched'
          : 'Legacy safeStorage orgs file did not decrypt — the OS credential store cannot read it (different machine or VDI session, or corruption), leaving it untouched',
        decryptError,
      );
      return null;

[paustint]

Went through Copilot's suppressed (low-confidence) comments — one was valid:

• apps/jetstream-desktop/src/services/persistence.service.ts:506 — fixed. A legacy orgs.json that never decrypts is never migrated and never rewritten, so it would keep its pre-SECURE_FILE_MODE mode bits forever. Added a best-effort chmod to 0600 on that path (contents still untouched, and the owning account reads it just the same), plus a test.

The earlier suppressed findings (the bindOrgStorageToUser error wording, the misleading decrypt-failure log, and the LEGACY_SFDC_ORGS_FILE "read once per user" comment) were all already addressed in later commits.

[Copilot]

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:423

• readOrgs() now returns empty any time org storage is not yet bound (ORG_SESSION is null). This means that on cold start where checkAuth hits a network error (and intentionally keeps the cached session), the app will still have a valid userProfile/accessToken but orgs/groups will remain empty even for installs that still have a legacy safeStorage orgs.json that could be decrypted locally without the portable key. Confirm this loss of offline/local-read behavior is intentional, since it can make the desktop app appear "logged in" but with no org state until the server is reachable again.

    const session = ORG_SESSION;
    // No authenticated user yet (cold start before auth completes). There is nothing to read
    // and nothing can be decrypted without the key — the caller retries once auth lands.
    if (!session) {
      logger.info('Orgs requested before org storage is bound to a user — returning empty');
      return { jetstreamOrganizations: [], salesforceOrgs: [] };
    }

[paustint]

Went through Copilot's suppressed (low-confidence) comments across all the review rounds — one was worth acting on:

• persistence.service.ts:418 — the readOrgs unbound-guard comment claimed "nothing can be decrypted without the key," which is not true for a legacy safeStorage orgs.json (machine-scoped, not user-scoped). Rewrote it to say the real reason: reading that file with no user bound would hand one account's orgs to whoever launched the app, which is the shared-file bug this scoping closes. Confirming Copilot's question — the loss of that offline read is intentional.

The rest were already handled: the .corrupt-* and legacy-file chmod findings landed via the posted threads, and the misleading bind error, the safeStorage-vs-portable decrypt log, and the "read once per user" comment on LEGACY_SFDC_ORGS_FILE were all fixed in earlier rounds.

[Copilot]

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:35

• This comment says the legacy orgs file is retired once its contents are "confirmed" in the scoped file, but the migration flow only checks that the scoped file exists (it doesn’t verify the decrypted contents match) before renaming the legacy file. Either add a verification step or adjust the comment to match the current behavior so future maintainers don’t assume stronger guarantees than implemented.

 * subsequent read, not just once. It is retired to a timestamped backup once its contents are
 * confirmed in the scoped file; see `adoptLegacyOrgsFile` and `retireLegacyOrgsFile`.

[Copilot]

Pull request overview

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

Suppressed comments (2)

apps/jetstream-desktop/src/services/ipc.service.ts:482

• Same as the login path: encryptionKey is mandatory on a successful /desktop-app/auth/verify response per AuthResponseSuccessSchema, so the conditional check is redundant. Binding unconditionally also makes it harder to end up in a “signed in but org storage not bound” state if the response shape changes unexpectedly.

      if (successResponse.encryptionKey) {
        dataService.bindOrgStorageToUser({ userId: successResponse.userProfile.id, encryptionKey: successResponse.encryptionKey });
      }

apps/jetstream-desktop/src/services/ipc.service.ts:152

• successResponse.encryptionKey is required by AuthResponseSuccessSchema (z.string().length(64)), so this conditional is redundant and can hide future contract regressions (the app would stay signed in but org storage would remain unbound). Bind org storage unconditionally on the success path.

This issue also appears on line 480 of the same file.

        if (successResponse.encryptionKey) {
          dataService.bindOrgStorageToUser({ userId: successResponse.userProfile.id, encryptionKey: successResponse.encryptionKey });
        }

[paustint]

Copilot's suppressed comments this round were both the same finding, and both were right:

• ipc.service.ts:150 and ipc.service.ts:480 — removed the if (successResponse.encryptionKey) guards. AuthResponseSuccessSchema requires z.string().length(64) and parseAuthResponse returns a failure on schema mismatch, so a success response always carries the key — the conditional was dead, and a 64-char non-hex value is truthy anyway, so it would already reach bindOrgStorageToUser and throw. Removing it is a runtime no-op that stops a contract regression from turning into a silent "signed in but org storage unbound".

Also closed out the persistence.service.ts:35 suppressed comment from the previous round: it correctly caught that the doc comment claimed retirement waits until the contents are "confirmed" in the scoped file when the code only checks existence. Reworded rather than adding a verification step — existence is sound because the migrating write is atomic (all-or-nothing), and retirement renames rather than deletes, so even the non-atomic fallback failing halfway leaves the data recoverable.

[Copilot]

Pull request overview

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