Swap stored password for Matrix access token (CS-10725)#4779
Draft
FadhlanR wants to merge 4 commits into
Draft
Conversation
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.
5c29c91 to
0a1267c
Compare
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
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
matrixAccessToken,matrixUserId,matrixDeviceIdinstead of the rawpassword. The password is only ever held on the stack during the initialprofile add(or re-auth) and never lands on disk.matrixLoginon every realm operation (refreshServerToken,addToUserRealms, etc.) now read the stored token via a new syncgetStoredMatrixAuth. The realm-server JWT continues to be derived from the stored access token via Matrix OpenID, exactly as before.migrateLegacyProfilesruns once per CLI invocation (rootpreActionhook). 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.reAuthenticatehandles 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, andgetActiveCredentialsare removed. The runtimeMATRIX_PASSWORDenv-var fallback is gone;MATRIX_PASSWORDis still read byprofile migratefor the one-time conversion only.Linear
CS-10725
Test plan
pnpm test:unit-exclude-smoke— 159/159 green)pnpm exec tsc --noEmit -p .)rg "password" packages/boxel-cli/srcaudit — only as a function arg or transient local, never persistedpnpm test:integration)boxel profile addwrites new token fields, nopasswordon diskprofiles.jsonis rewritten on first command🤖 Generated with Claude Code