diff --git a/.github/workflows/service-aliases.yml b/.github/workflows/service-aliases.yml index fe431be5..6e0137d1 100644 --- a/.github/workflows/service-aliases.yml +++ b/.github/workflows/service-aliases.yml @@ -56,6 +56,8 @@ jobs: run: >- uv run --locked --extra dev python -m pytest tests/test_service_aliases.py tests/test_client_connection.py + tests/test_client_accounts.py tests/test_client_account_recovery.py + tests/test_client_paths.py tests/test_hosting_alias_ingress.py tests/test_backup_daemon.py tests/test_config_safety.py tests/test_configuration_field_contracts.py tests/test_execution_composition.py tests/test_cli_cursor_output.py diff --git a/docs/CLIENT_ACCOUNT_RECOVERY_PROGRESS.md b/docs/CLIENT_ACCOUNT_RECOVERY_PROGRESS.md new file mode 100644 index 00000000..3f982263 --- /dev/null +++ b/docs/CLIENT_ACCOUNT_RECOVERY_PROGRESS.md @@ -0,0 +1,43 @@ +# Account selection and recovery — issue #219 + +Source baseline: `4bbdbca52b54f8d2de36a7a04b5207a9b9951d61` (msgctl 0.2.11). +Scope: client account selection, metadata listing and legacy migration boundaries. + +## Reproductions and changes + +- Restored account directories without `current-account.json` silently fall back + to `default`, even when that is a different saved identity. Require an explicit + selection when saved accounts exist and no legacy service singleton remains. +- A selected account with missing state can accidentally import a different old + profile or create an empty replacement. Reject the missing selected state before + migration; apply the legacy-handle guard to saved selections as well as explicit + `--account` arguments. Explicit import remains available for recovery. +- `account list` creates state, changes selection and can move legacy credentials. + Inspect config/data/state account directories and safe metadata without creating + files, loading keys or migrating anything. Show incomplete accounts as well. +- `account use` can update the remembered service before rejecting an + unauthenticated account. Check the saved subject before constructing client + state. Failed selection preserves the previous service and account. +- The list's `selected` flag now reflects an invocation-only `--account` override, + while the persistent default remains unchanged. + +## Validation + +`tests/test_client_account_recovery.py` adds regression coverage. Existing account, +connection and path tests continue to cover singleton migration, profile aliases, +hardware/software separation, imports, keys and journals. The service-alias CI +workflow includes all these files and records source and JUnit evidence. + +The first PR commit contains only tests and workflow wiring, for baseline failure +evidence. The following implementation commit is verified separately. Exact run +results belong in PR #232; this file does not predeclare CI success. + +The unchanged baseline produced nine account-regression failures in service-alias +run `36907335525`. Its tenth failure is an existing branding assertion, also +present in main run `36904405959`: it requires copy from the retired landing page. +The smoke test now checks the exact bundled root-page bytes, the 3D canvas and the +accessible catalog, retaining its CSP, font, language and HTTP assertions. + +Issue #219 also includes server-account retirement and full recovery acceptance. +Those are separate from this code change; the issue stays open. No production +account, credential, release or deployment is changed by this work. diff --git a/docs/FILESYSTEM_LAYOUT.md b/docs/FILESYSTEM_LAYOUT.md index 716816ec..8d4c560c 100644 --- a/docs/FILESYSTEM_LAYOUT.md +++ b/docs/FILESYSTEM_LAYOUT.md @@ -30,6 +30,10 @@ Local names allow letters, digits, underscores, dots and hyphens, begin with a l The service default is stored in `~/.config/msg/services//current-account.json`. An invocation override does not change it. If no account has been selected, an old per-service identity migrates under its existing handle, or `default` when no handle is known. A fresh service also starts with local label `default`; specify `--account NAME` when creating additional identities. Existing `hardware-signer.json` is account state, not a private key. Its path is `~/.local/state/msg/services//accounts//hardware-signer.json`. +If account directories have been restored without the selection record and no old per-service identity remains, ordinary commands stop with `local_account_selection_required`. Run `msg account list`, then explicitly choose `msg account use NAME` or the invocation-only `--account NAME`. A selection pointing to a missing `client.json` stops with `local_account_not_found`; it never silently creates an empty replacement or imports a different legacy identity. An explicit account import can restore the missing state. + +`account list` only inspects local metadata. It does not create directories, select a default, load private keys, contact the service or migrate an old profile. Partially restored accounts remain visible with missing metadata set to null. Its `selected` field reflects an explicit `--account` override for this invocation; otherwise it reflects the saved service default. A rejected `account use` on an unregistered account keeps both the previous service and account selection. + Stop old clients and background listeners before upgrading or migrating. Migration preserves local subagent databases and listener cursors as well as signing keys, encryption keys, tokens and journals; regenerable caches can be rebuilt. Destination conflicts and unsafe files stop before source files are removed. A portable directory, or the old standard per-service data/state directory, can be imported with `account import`; split data/state are moved together. No `msg-hardware` namespace is used. Stop clients using the old profile before migrating: diff --git a/src/msg/client.py b/src/msg/client.py index 37ff7437..a109c625 100644 --- a/src/msg/client.py +++ b/src/msg/client.py @@ -163,6 +163,17 @@ def __init__( require(account_selection.get('version') == 1, 'unknown_client_state_version') ClientPaths.discover(server=server, account=account_selection.get('account', '')) old_service = private_client_json(service_paths.state / 'client.json') + if ( + account is None + and account_selection is None + and old_service is None + and migrate_from is None + ): + require( + not service_paths.account_names(), + 'local_account_selection_required', + details={'options': ['account list', 'account use NAME', '--account NAME']}, + ) self.account = ( account if account is not None @@ -173,16 +184,21 @@ def __init__( ) ) self.paths = ClientPaths.discover(server=server, account=self.account) + if account is None and account_selection is not None and migrate_from is None: + require( + private_client_json(self.paths.state / 'client.json') is not None, + 'local_account_not_found', + ) if ( old_service is not None and migrate_from is None and (self.account == old_service.get('handle') or self.account == 'default') ): legacy = service_paths - if account is not None and legacy is not None and migrate_from is None: + if legacy is not None and migrate_from is None: source_state = legacy.state if isinstance(legacy, ClientPaths) else legacy source_data = private_client_json(source_state / 'client.json') or {} - if account not in {'default', source_data.get('handle')}: + if self.account not in {'default', source_data.get('handle')}: legacy = None if legacy is not None and migrate_from is None: current = private_client_json(self.paths.state / 'client.json') diff --git a/src/msg/client_accounts.py b/src/msg/client_accounts.py index c541df0c..01664047 100644 --- a/src/msg/client_accounts.py +++ b/src/msg/client_accounts.py @@ -10,6 +10,21 @@ from msg.service_origin import service_origin +def selected_service(args): + previous = ClientPaths.discover(profile=args.profile) + selection = private_client_json(previous.config / 'service.json') + if selection is not None: + require(selection.get('version') == 1, 'unknown_client_state_version') + server = args.server or os.environ.get('MSG_SERVER') or (selection or {}).get('server') + if server is None: + legacy = private_client_json(previous.state / 'client.json') + if legacy is None: + legacy = private_client_json(previous.config / 'client.json') + server = (legacy or {}).get('server') + require(server is not None, 'server_required') + return service_origin(server) + + def run_command(args): require(args.config_dir is None, 'account_conflicts_with_config_dir') if args.action == 'import': @@ -30,48 +45,47 @@ def run_command(args): 'handle': state.data.get('handle'), } if args.action == 'use': - previous = ClientPaths.discover(profile=args.profile) - server = ( - args.server - or os.environ.get('MSG_SERVER') - or (private_client_json(previous.config / 'service.json') or {}).get('server') - ) - require(server is not None, 'server_required') - server = service_origin(server) + server = selected_service(args) paths = ClientPaths.discover(server=server, account=args.name) saved = private_client_json(paths.state / 'client.json') require(saved is not None, 'local_account_not_found') + require(saved.get('subject_id') is not None, 'local_account_not_authenticated') selected = ClientState(server=server, profile=args.profile, account=args.name) require(selected.subject is not None, 'local_account_not_authenticated') marker = ClientPaths.discover(server=server).config / 'current-account.json' private_directory(marker.parent) durable_write(marker, canonical({'version': 1, 'account': args.name}), mode=0o600) return {'account': args.name, 'server': server, 'handle': selected.data.get('handle')} - # Resolve the selected service and safely migrate its old singleton layout. - state = ClientState(server=args.server, profile=args.profile, account=args.account) - service = ClientPaths.discover(server=state.server) + server = selected_service(args) + service = ClientPaths.discover(server=server) marker = service.config / 'current-account.json' - selection = private_client_json(marker) or {'account': 'default'} - root = service.state / 'accounts' + selection = private_client_json(marker) + if selection is not None: + require(selection.get('version') == 1, 'unknown_client_state_version') + ClientPaths.discover(server=server, account=selection.get('account', '')) + names = service.account_names() + selected = (selection or {}).get('account') + if args.account is not None: + ClientPaths.discover(server=server, account=args.account) + require(args.account in names, 'local_account_not_found') + selected = args.account result = [] - for directory in sorted(root.iterdir()): - require(not directory.is_symlink(), 'unsafe_client_directory') - if not directory.is_dir(): - continue - # Validate each account name and directory without loading/exporting keys. - paths = ClientPaths.discover(server=state.server, account=directory.name) + for name in names: + paths = ClientPaths.discover(server=server, account=name) saved = private_client_json(paths.state / 'client.json') - if saved is None: - continue + if saved is not None: + require(saved.get('version') == 1, 'unknown_client_state_version') + require(service_origin(saved.get('server')) == server, 'client_server_mismatch') + saved = saved or {} result.append({ - 'account': directory.name, + 'account': name, 'handle': saved.get('handle'), 'subject_id': saved.get('subject_id'), - 'selected': directory.name == selection['account'], + 'selected': name == selected, 'signer': 'yubikey' if paths.file('hardware-signer.json').exists() else 'software' if paths.file('identity.key').exists() else None, }) - return {'server': state.server, 'accounts': result} + return {'server': server, 'accounts': result} diff --git a/src/msg/paths.py b/src/msg/paths.py index 3a8f753f..d2181d21 100644 --- a/src/msg/paths.py +++ b/src/msg/paths.py @@ -118,6 +118,33 @@ def prepare(self): private_directory(path.parent.parent) private_directory(path) + def account_names(self): + """Inspect a service's saved accounts without creating or migrating files.""" + names = set() + for base in {self.config, self.data, self.state}: + root = base / 'accounts' + for parent in (root, *root.parents): + require(not parent.is_symlink(), 'unsafe_client_directory') + if not root.exists(): + continue + info = root.stat() + require( + stat.S_ISDIR(info.st_mode) and info.st_uid == os.geteuid(), + 'unsafe_client_directory', + ) + for entry in root.iterdir(): + info = entry.lstat() + require(not stat.S_ISLNK(info.st_mode), 'unsafe_client_directory') + if not stat.S_ISDIR(info.st_mode): + continue + require( + info.st_uid == os.geteuid() and info.st_mode & 0o077 == 0, + 'unsafe_client_directory', + ) + self.discover(account=entry.name) + names.add(entry.name) + return sorted(names) + def temporary_parent(self): value = os.environ.get('XDG_RUNTIME_DIR') if not value or not Path(value).is_absolute(): diff --git a/tests/test_branding.py b/tests/test_branding.py index d6a993a5..309a3ed7 100644 --- a/tests/test_branding.py +++ b/tests/test_branding.py @@ -9,6 +9,7 @@ import pytest from test_service import NOW, call, register +from msg.bootstrap import ROOT_WEB_SAMPLE from msg.core.codec import b64, canonical, wire from msg.core.requests import request_for from msg.transports.dictionary import build_dictionary @@ -81,7 +82,9 @@ async def test_logo_is_packaged_and_served_read_only(installed): hosted = await http.get('/@root/web/index.html') assert hosted.status_code == 200 and b'' in hosted.text - assert 'a conversation begins.' in hosted.text + assert hosted.content == ROOT_WEB_SAMPLE + assert '