fix: Swap stored password for Matrix access token (CS-10725) by FadhlanR · Pull Request #4779 · cardstack/boxel

FadhlanR · 2026-05-12T07:52:36Z

Summary

boxel-cli profiles now persist matrixAccessToken, matrixUserId, matrixDeviceId instead of the raw password. The password is only ever held on the stack during the initial profile add (or re-auth) and never lands on disk.
All callers that used to re-run matrixLogin on every realm operation (refreshServerToken, addToUserRealms, etc.) now read the stored token via a new sync getStoredMatrixAuth. The realm-server JWT continues to be derived from the stored access token via Matrix OpenID, exactly as before.
A new migrateLegacyProfiles runs once per CLI invocation (root preAction hook). For any profile still on the pre-CS-10725 schema it does one login with the stored password and replaces the field with tokens. Per-profile failures log a warning and leave the profile alone.
A new reAuthenticate handles the case where Matrix later rejects the stored token (401/403). On a TTY it prompts for the password and refreshes the token transparently; off-TTY it surfaces a "re-add the profile" error.
getPassword, updatePassword, and getActiveCredentials are removed. The runtime MATRIX_PASSWORD env-var fallback is gone; MATRIX_PASSWORD is still read by profile migrate for the one-time conversion only.

Linear

CS-10725

Test plan

Unit tests pass (pnpm test:unit-exclude-smoke — 159/159 green)
Type check passes (pnpm exec tsc --noEmit -p .)
rg "password" packages/boxel-cli/src audit — only as a function arg or transient local, never persisted
Integration tests against real Synapse (pnpm test:integration)
Manual smoke 1 (fresh profile): boxel profile add writes new token fields, no password on disk
Manual smoke 2 (auto-migration): hand-crafted legacy profiles.json is rewritten on first command
Manual smoke 3 (401 re-auth): revoke device in Synapse, next command prompts for password and recovers

🤖 Generated with Claude Code

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0a1267c7e8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-05-13T11:39:27Z

+    await manager.addProfile(
+      matrixId,
+      password,
+      displayName,
+      matrixUrl,


Invalidate cached tokens before rewriting profile URLs

When profile add updates an existing profile, this now calls manager.addProfile(...) before updateUrls(...). addProfileWithAuth preserves the previous realmTokens and realmServerToken, and because URLs have already been rewritten by the time updateUrls runs, it sees no change and does not clear those caches. That leaves tokens minted for the old server in the profile; code paths like realm create that use getOrRefreshServerToken() + fetchAndStoreRealmToken() without an automatic 401 retry can fail or silently skip token acquisition after a URL change.

Useful? React with 👍 / 👎.

Fixed in e830ad1. addProfileWithAuth now compares the resolved URLs against the existing profile and clears realmTokens/realmServerToken when either URL changes. Independently, addProfile defaults omitted URL args to the existing profile so the no-flag re-auth path no longer rewrites URLs at all.

chatgpt-codex-connector · 2026-05-13T11:39:27Z

      matrixUrl: matrixUrl || defaultMatrixUrl,
      realmServerUrl: realmServerUrl || defaultRealmUrl,
-      password,
+      displayName: displayName || `${username} \u00b7 ${domain}`,


Preserve custom display names during profile re-auth

The update path now always goes through addProfile, and when --name is omitted it passes undefined for displayName. This fallback unconditionally recomputes the default <username> · <domain> name, so re-adding an existing profile (or env migration refresh) overwrites any user-customized display name. Previously profile updates kept the stored name unless a new one was explicitly provided.

Useful? React with 👍 / 👎.

Fixed in e830ad1. addProfile now falls back to the existing profile's displayName when none is passed, so re-auth without --name preserves the user-customised name. Added a unit test covering this in tests/commands/profile.test.ts.

jurgenwerk · 2026-05-13T11:42:30Z

nice to see this–is the matrix token long lived and does not refresh?

Copilot

Pull request overview

This PR updates boxel-cli profile authentication to stop persisting raw Matrix passwords on disk, storing Matrix access-token credentials instead, with automatic legacy-profile migration and a re-authentication flow when tokens are revoked.

Changes:

Replace persisted password in profiles with { matrixAccessToken, matrixUserId, matrixDeviceId }, and add one-shot migration from legacy profiles.
Add interactive re-authentication when Matrix rejects a stored token (401/403), and update Matrix-calling paths to reuse stored auth with recovery.
Move happy-path profile add subprocess smoke coverage into integration tests backed by the test Synapse + realm-server harness.

Reviewed changes

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

Show a summary per file

File	Description
packages/boxel-cli/tests/smoke.test.ts	Removes network-bound happy-path `profile add` smoke assertions; keeps only pre-Matrix validation paths.
packages/boxel-cli/tests/lib/auth-resolver.test.ts	Updates tests to seed profiles via token-based auth (`addProfileWithAuth`).
packages/boxel-cli/tests/integration/profile-add.test.ts	New integration subprocess tests validating `profile add` persists tokens (not password) and honors `--quiet`.
packages/boxel-cli/tests/helpers/integration.ts	Exports test Matrix credentials and adds a test-only mitigation for unhandled job rejections; updates JWT test-profile seeding.
packages/boxel-cli/tests/commands/profile.test.ts	Updates unit tests to use injected/stubbed Matrix login and adds coverage for migration + re-auth flows.
packages/boxel-cli/tests/commands/profile-env-resolution.test.ts	Adds focused unit coverage for BOXEL_ENVIRONMENT slug/URL derivation helpers.
packages/boxel-cli/src/lib/profile-manager.ts	Implements token persistence, legacy migration, stored-auth access, and re-auth retry logic; removes password/env credential getters.
packages/boxel-cli/src/lib/auth.ts	Introduces `MatrixAuthError` and uses it for 401/403 token rejection in key Matrix calls.
packages/boxel-cli/src/commands/profile.ts	Exposes env-resolution helpers for unit tests and updates `profile add` non-interactive flow to use token-based storage.
packages/boxel-cli/src/build-program.ts	Adds a CLI startup hook intended to migrate legacy profiles once per invocation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

FadhlanR · 2026-05-15T09:24:50Z

+    .hook('preAction', async (thisCommand) => {
      let opts = thisCommand.optsWithGlobals?.() ?? thisCommand.opts();
      if (opts.quiet) {
        setQuiet(true);
      }
      warnIfMisplacedLocalRealmDirs(process.cwd());
+      // One-shot migration for profiles persisted before CS-10725 (when the
+      // schema stored `password` instead of `matrixAccessToken`). Runs once
+      // per CLI invocation: re-logs-in with the on-disk password and
+      // replaces it with the resulting access token. Failures are warned
+      // about and skipped so a single broken profile doesn't block the
+      // rest of the command.
+      try {
+        await getProfileManager().migrateLegacyProfiles();
+      } catch {


Resolved in e830ad1 by removing the async work from the hook. Per @jurgenwerk's suggestion below, migrateLegacyProfiles is gone entirely; the preAction hook is sync again, so program.parse() remains correct.

FadhlanR · 2026-05-15T09:24:59Z

+  const isUpdate = Boolean(manager.getProfile(matrixId));
+
+  // addProfile performs a real matrixLogin and persists the resulting
+  // access token (the password never lands on disk). It also handles the
+  // create-vs-reauth split uniformly: re-running it on an existing profile
+  // refreshes the stored token while preserving cached realm tokens.
+  try {
+    await manager.addProfile(
+      matrixId,
+      password,
+      displayName,
+      matrixUrl,
+      realmServerUrl,
    );


Fixed in e830ad1. addProfile now defaults omitted matrixUrl/realmServerUrl to the existing profile's stored values before calling resolveProfileSlots, so re-running profile add -u <id> -p <pw> against an existing custom-domain profile no longer throws "Unknown domain" or resets the URLs to defaults. Covered by a new test.

FadhlanR · 2026-05-15T09:25:08Z

@@ -225,11 +251,42 @@ export class ProfileManager implements RealmAuthenticator {
        : 'https://realms-staging.stack.cards/';



Fixed in e830ad1. resolveProfileSlots now has an explicit local branch that returns http://localhost:8008 / http://localhost:4201/, matching MENU_ENVIRONMENTS.local. Added a unit test for @user:localhost.

FadhlanR · 2026-05-15T09:25:19Z

+    if (!profile.matrixAccessToken) {
+      throw new Error(
+        `Profile "${targetId}" has no stored Matrix access token. ` +
+          `Run \`boxel profile add ${targetId}\` to authenticate.`,


Updated in e830ad1 to Run ''boxel profile add'' to re-authenticate. — pointing users at the interactive flow keeps the message short and avoids exposing -u/-p syntax (and -p would be discouraged anyway since it leaks via shell history; BOXEL_PASSWORD is the preferred non-interactive route).

FadhlanR · 2026-05-15T09:25:27Z

+    for (const [id, profile] of Object.entries(this.config.profiles)) {
+      if (profile.matrixAccessToken || !profile.password) {
+        continue;
+      }
+      try {
+        const username = getUsernameFromMatrixId(id);
+        const auth = await this.matrixLoginFn(
+          profile.matrixUrl,
+          username,
+          profile.password,
+        );
+        profile.matrixAccessToken = auth.accessToken;
+        profile.matrixUserId = auth.userId;
+        profile.matrixDeviceId = auth.deviceId;
+        delete profile.password;


Resolved in e830ad1 by dropping migrateLegacyProfiles entirely (see @jurgenwerk's suggestion below). Any pre-CS-10725 profile now hits the friendly "re-authenticate" error from getStoredMatrixAuth instead, which prompts the user to re-run boxel profile add.

FadhlanR · 2026-05-15T09:25:39Z

  async removeFromUserRealms(realmUrl: string): Promise<boolean> {
-    let matrixAuth = await this.loginToMatrix();
-    return removeRealmFromMatrixAccountData(matrixAuth, realmUrl);
+    return this.withMatrixAuthRecovery((auth) =>
+      removeRealmFromMatrixAccountData(auth, realmUrl),
+    );
  }


Fixed in e830ad1. removeRealmFromMatrixAccountData now throws MatrixAuthError on 401/403, mirroring addRealmToMatrixAccountData. Added an end-to-end test that runs removeFromUserRealms against a fetch stub returning 401 on the first PUT and verifies the call recovers via withMatrixAuthRecovery + reAuthenticate.

jurgenwerk · 2026-05-13T11:55:40Z

+  // `password` field but no `matrixAccessToken`, perform a real Matrix login
+  // and replace the password with the resulting tokens. Failures are warned
+  // about and left in place so the user can re-add the profile themselves.
+  async migrateLegacyProfiles(): Promise<{


Given so few people used the cli perhaps we can just throw an error with the message "please re-add your profile" in case the profile is present but the matrix access token isn't present? So that we don't maintain legacy code in a relatively fresh project

Agreed — done in e830ad1. Dropped migrateLegacyProfiles, removed the legacy password? field from the Profile type, and removed the preAction hook that ran the migration. Any leftover pre-CS-10725 profile now lands in getStoredMatrixAuth's friendly "no stored Matrix access token. Run ''boxel profile add'' to re-authenticate." error.

FadhlanR · 2026-05-13T11:57:36Z

nice to see this–is the matrix token long lived and does not refresh?

In the current implementation, we use long-lived token, however, there is an option to enable a refresh token flow.

boxel-cli profiles used to persist the user's Matrix password in ~/.boxel-cli/profiles.json and re-run matrixLogin on every realm operation. This change replaces the on-disk `password` field with the three values matrixLogin already returns — `matrixAccessToken`, `matrixUserId`, `matrixDeviceId` — so the password is only ever held on the stack during the initial login (or re-auth) and never lands on disk. - New ProfileManager.addProfileWithAuth(matrixId, MatrixAuth, ...) is the low-level store half; addProfile calls matrixLogin once and then delegates. Re-running addProfile on an existing profile refreshes the token while preserving cached realm tokens. - New getStoredMatrixAuth (sync) replaces the private loginToMatrix. refreshServerToken, getOrRefreshServerToken, addToUserRealms, removeFromUserRealms, and getUserRealms all read the stored token instead of re-running login. - New reAuthenticate handles 401 from Matrix: on TTY it prompts for the password, re-runs matrixLogin, and persists the new tokens. Non-TTY surfaces "run `boxel profile add` to re-authenticate". Wired into refreshServerToken and the user-realms helpers. - auth.ts throws a typed MatrixAuthError on 401/403 so callers can drive recovery without parsing messages. - New migrateLegacyProfiles runs once per CLI invocation (root preAction hook in build-program.ts). It finds any profile with the pre-CS-10725 `password` field, logs in once, writes the resulting tokens, and deletes the password. Per-profile failures are warned about so a single broken profile doesn't block the rest. - migrateFromEnv now goes through addProfile (which does the real login) so the env-var path also avoids storing the password. - Removed getPassword, updatePassword, getActiveCredentials. The runtime env-var fallback (MATRIX_PASSWORD etc.) is gone; MATRIX_PASSWORD is still read by `profile migrate` for the one-time conversion only. - Integration helpers: setupTestProfile still goes through addProfile (real Synapse login). setupJwtTestProfile switches to addProfileWithAuth with a fake MatrixAuth. ProfileManager now accepts an optional `deps` object (matrixLogin, promptPassword, isTty) so unit tests drive the auth seams without touching a real Matrix server. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The smoke suite at tests/smoke.test.ts had been doing two jobs: it checked CLI surface behaviour (flag validation, BOXEL_ENVIRONMENT sanitisation, "unknown domain" guards) AND it asserted that successful `profile add` invocations wrote the right URLs to profiles.json. Pre-CS-10725 the latter ran network-free; after this PR `addProfile` does a real Matrix login, so happy-path smoke tests either rate-limited matrix-staging.stack.cards (429 in CI) or failed DNS on synthetic *.my.server URLs. Restructure to keep the two jobs apart: - tests/smoke.test.ts now only exercises paths that fail before any Matrix call: --matrix-url / --realm-server-url validation, BOXEL_ENVIRONMENT leak prevention, "Unknown domain" without flags, and slugifies-to-empty. - tests/integration/profile-add.test.ts (new) subprocesses the built CLI binary against the dockerised Synapse + realm-server that the rest of the integration suite already spins up. Covers the happy-path success line under --quiet and without, explicit URL flags, whitespace trimming, --matrix-url / --realm-server-url override of BOXEL_ENVIRONMENT, and (the new property after this PR) re-running `profile add` refreshes the stored access token. - tests/commands/profile-env-resolution.test.ts (new) unit-tests computeEnvSlug and resolveBoxelEnvironment directly, preserving the env-slug coverage that previously lived in two subprocess smoke tests. Exports added so the new tests can reach internals: - TEST_USERNAME / TEST_PASSWORD from tests/helpers/integration.ts so the integration subprocess test can authenticate as the cli-test user that startTestRealmServer already registers. - computeEnvSlug / resolveBoxelEnvironment from commands/profile.ts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- packages/boxel-cli/tests/commands/profile.test.ts: prettier autofix for 6 wrapping issues introduced by the test rewrite. No behaviour changes. - packages/boxel-cli/plugin/skills/realm-sync/SKILL.md: regenerated via `pnpm build:plugin`. The synopsis had been stale since the CS-10624 (realm watch stop) merge — that change turned `realm watch` into a subcommand parent but the markdown still documented the pre-CS-10624 args. Unrelated to this PR's scope but blocking CI on `verify-plugin-fresh`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

removeRealm triggers cancelRunningJobsInConcurrencyGroup on the realm- server, which rejects every coalesced Deferred for the realm's indexing job. server.createRealm / handle-publish-realm / full-reindex discard the Job they get from enqueueReindexRealmJob, leaving an orphan Deferred whose rejection surfaces to vitest as an unhandled error and fails the suite even though all assertions pass. Wrap publish() on the test-process queue publisher so every returned Job.done gets a no-op catch handler; real consumers (publishFullIndex's chain) still observe the rejection through their own handlers. Upstream fix belongs in runtime-common's enqueueReindexRealmJob and is out of scope here.

@jurgenwerk

- Drop migrateLegacyProfiles entirely (per @jurgenwerk) and remove the legacy `password?` field from the Profile type. getStoredMatrixAuth's error message now guides the user to `boxel profile add` for re-auth, serving any pre-CS-10725 profile still on disk. - resolveProfileSlots gets an explicit `local` branch so `@user:localhost` non-interactive profiles default to http://localhost:8008 / http://localhost:4201/ instead of staging. - addProfile preserves the existing profile's displayName and URLs when the corresponding args are omitted on re-auth, fixing the regression where re-add without --name/--matrix-url/--realm-server-url silently reset those fields to domain-derived defaults. - addProfileWithAuth now clears cached realmTokens/realmServerToken when the resolved URLs differ from the existing profile's URLs, so tokens minted for the old server can't linger after a URL change. - removeRealmFromMatrixAccountData throws MatrixAuthError on 401/403 (mirroring addRealmToMatrixAccountData), so withMatrixAuthRecovery in removeFromUserRealms can drive interactive re-auth on a revoked token. - build-program.ts: drop the migrateLegacyProfiles invocation and revert the preAction hook to sync (no more async work to await). - Tests: delete the migrateLegacyProfiles describe block; add coverage for the legacy-profile error path, displayName/URL preservation, URL-change cache invalidation, localhost defaults, and the removeFromUserRealms 401-recovery flow. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

backspace · 2026-05-19T19:52:51Z

A new migrateLegacyProfiles runs once per CLI invocation (root preAction hook). For any profile still on the pre-CS-10725 schema it does one login with the stored password and replaces the field with tokens. Per-profile failures log a warning and leave the profile alone.

Is this true? I have stored profiles with passwords (😳) and it doesn’t seem to be migrating for me:

❯ ./dist/index.js profile

Active Profile: [buck · stack.cards]
  Display Name: buck · stack.cards
  Matrix URL: https://matrix-staging.stack.cards
  Realm Server: https://realms-staging.stack.cards/

Commands:
  boxel profile list      List all profiles
  boxel profile add       Add a new profile
  boxel profile switch    Switch active profile
  boxel profile remove    Remove a profile
  boxel profile migrate   Import from .env file
❯ ./dist/index.js realm list
Error: Failed to load UI realm list: Profile "@buck:stack.cards" has no stored Matrix access token. Run `boxel profile add` to re-authenticate.
❯ ./dist/index.js profile migrate

Migrate from .env

No complete credentials found in environment variables.

Required variables: MATRIX_URL, MATRIX_USERNAME, MATRIX_PASSWORD, REALM_SERVER_URL

FadhlanR force-pushed the cs-10725-swap-stored-password-for-stored-access-token branch from 5c29c91 to 0a1267c Compare May 13, 2026 04:55

FadhlanR marked this pull request as ready for review May 13, 2026 11:35

chatgpt-codex-connector Bot reviewed May 13, 2026

View reviewed changes

FadhlanR requested a review from a team May 13, 2026 11:40

jurgenwerk requested a review from Copilot May 13, 2026 11:41

Copilot started reviewing on behalf of jurgenwerk May 13, 2026 11:41 View session

Copilot AI reviewed May 13, 2026

View reviewed changes

jurgenwerk reviewed May 13, 2026

View reviewed changes

FadhlanR requested a review from jurgenwerk May 18, 2026 07:43

FadhlanR and others added 5 commits May 19, 2026 13:42

FadhlanR force-pushed the cs-10725-swap-stored-password-for-stored-access-token branch from e830ad1 to a26daca Compare May 19, 2026 06:42

FadhlanR changed the title ~~Swap stored password for Matrix access token (CS-10725)~~ fix: Swap stored password for Matrix access token (CS-10725) May 19, 2026

		@@ -225,11 +251,42 @@ export class ProfileManager implements RealmAuthenticator {
		: 'https://realms-staging.stack.cards/';

Conversation

FadhlanR commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Linear

Test plan

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot May 13, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jurgenwerk commented May 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FadhlanR commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

backspace commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

FadhlanR commented May 12, 2026 •

edited

Loading

FadhlanR commented May 13, 2026 •

edited

Loading