diff --git a/src/basic_memory/cli/commands/cloud/core_commands.py b/src/basic_memory/cli/commands/cloud/core_commands.py index c61c4eb8..595b13fc 100644 --- a/src/basic_memory/cli/commands/cloud/core_commands.py +++ b/src/basic_memory/cli/commands/cloud/core_commands.py @@ -28,6 +28,7 @@ from basic_memory.cli.commands.cloud.bisync_commands import ( ) from basic_memory.cli.commands.cloud.rclone_config import ( configure_rclone_remote, + rclone_remote_exists, remote_name_for_workspace, ) from basic_memory.cli.commands.cloud.rclone_installer import ( @@ -207,6 +208,11 @@ def setup( help="Set up sync for a specific workspace (slug, name, or tenant_id). " "Omit for your default workspace.", ), + force: bool = typer.Option( + False, + "--force", + help="Reconfigure an rclone remote that already exists (mints new credentials).", + ), ) -> None: """Set up cloud sync by installing rclone and configuring credentials. @@ -239,6 +245,20 @@ def setup( workspace_id = None # default tenant remote_name = remote_name_for_workspace(None, is_default=True) + # Trigger: the target rclone remote already exists. + # Why: re-running setup mints new credentials and overwrites the remote, + # which would silently repoint it — e.g. clobbering the shared + # basic-memory-cloud remote that served another tenant. Checked BEFORE + # minting so an abort wastes no credentials. + # Outcome: stop unless the user explicitly opts in with --force. + if rclone_remote_exists(remote_name) and not force: + console.print(f"[red]rclone remote '{remote_name}' is already configured.[/red]") + console.print( + "Re-running setup mints new credentials and overwrites it. " + "Pass --force to reconfigure." + ) + raise typer.Exit(1) + # Step 2: Get tenant info (scoped to the target workspace when given) console.print("\n[blue]Step 2: Getting tenant information...[/blue]") tenant_info = run_with_cleanup(get_mount_info(workspace_id=workspace_id)) diff --git a/tests/cli/cloud/test_project_sync_command.py b/tests/cli/cloud/test_project_sync_command.py index b1427c71..f3d8441c 100644 --- a/tests/cli/cloud/test_project_sync_command.py +++ b/tests/cli/cloud/test_project_sync_command.py @@ -625,17 +625,21 @@ def test_get_workspace_for_project_override_no_match_raises(monkeypatch, config_ assert "No accessible workspace matches 'acme'" in str(exc_info.value) -def test_cloud_setup_workspace_configures_named_remote(monkeypatch): - """`bm cloud setup --workspace acme` provisions the acme tenant's own remote.""" - core = importlib.import_module("basic_memory.cli.commands.cloud.core_commands") - recorder: dict = {} - +def _stub_setup_env(monkeypatch, core, *, remote_exists=False, recorder=None): + """Stub the `bm cloud setup` dependency chain for the acme workspace.""" monkeypatch.setattr(core, "install_rclone", lambda: None) monkeypatch.setattr( core, "get_available_workspaces", lambda: _async_value([_workspace("team-tenant", "organization", "acme")]), ) + monkeypatch.setattr(core, "rclone_remote_exists", lambda _remote: remote_exists) + + def _mint(_tenant_id): + if recorder is not None: + recorder["minted"] = True + return _async_value(SimpleNamespace(access_key="ak", secret_key="sk")) + monkeypatch.setattr( core, "get_mount_info", @@ -643,24 +647,74 @@ def test_cloud_setup_workspace_configures_named_remote(monkeypatch): SimpleNamespace(tenant_id="team-tenant", bucket_name="acme-bucket") ), ) - monkeypatch.setattr( - core, - "generate_mount_credentials", - lambda _tenant_id: _async_value(SimpleNamespace(access_key="ak", secret_key="sk")), - ) + monkeypatch.setattr(core, "generate_mount_credentials", _mint) def _fake_configure(**kwargs): - recorder.update(kwargs) + if recorder is not None: + recorder.update(kwargs) return kwargs.get("remote_name") monkeypatch.setattr(core, "configure_rclone_remote", _fake_configure) + +def test_cloud_setup_workspace_configures_named_remote(monkeypatch): + """`bm cloud setup --workspace acme` provisions the acme tenant's own remote.""" + core = importlib.import_module("basic_memory.cli.commands.cloud.core_commands") + recorder: dict = {} + _stub_setup_env(monkeypatch, core, remote_exists=False, recorder=recorder) + result = runner.invoke(app, ["cloud", "setup", "--workspace", "acme"]) assert result.exit_code == 0, result.output assert recorder["remote_name"] == "basic-memory-cloud-acme" +def test_cloud_setup_aborts_when_remote_exists_without_force(monkeypatch): + """Setup refuses to overwrite an existing remote, and mints nothing (the #922 footgun).""" + core = importlib.import_module("basic_memory.cli.commands.cloud.core_commands") + recorder: dict = {} + _stub_setup_env(monkeypatch, core, remote_exists=True, recorder=recorder) + + result = runner.invoke(app, ["cloud", "setup", "--workspace", "acme"]) + + assert result.exit_code == 1, result.output + output = " ".join(result.output.split()) + assert "basic-memory-cloud-acme' is already configured" in output + assert "--force" in output + # Aborted before minting credentials or touching the remote. + assert "minted" not in recorder + assert "remote_name" not in recorder + + +def test_cloud_setup_force_overwrites_existing_remote(monkeypatch): + """--force reconfigures an existing remote.""" + core = importlib.import_module("basic_memory.cli.commands.cloud.core_commands") + recorder: dict = {} + _stub_setup_env(monkeypatch, core, remote_exists=True, recorder=recorder) + + result = runner.invoke(app, ["cloud", "setup", "--workspace", "acme", "--force"]) + + assert result.exit_code == 0, result.output + assert recorder["remote_name"] == "basic-memory-cloud-acme" + assert recorder.get("minted") is True + + +def test_cloud_setup_default_workspace_aborts_when_remote_exists(monkeypatch): + """The original footgun: `bm cloud setup` (no --workspace) must not clobber + the shared basic-memory-cloud remote without --force.""" + core = importlib.import_module("basic_memory.cli.commands.cloud.core_commands") + recorder: dict = {} + _stub_setup_env(monkeypatch, core, remote_exists=True, recorder=recorder) + + result = runner.invoke(app, ["cloud", "setup"]) # no --workspace → basic-memory-cloud + + assert result.exit_code == 1, result.output + output = " ".join(result.output.split()) + assert "'basic-memory-cloud' is already configured" in output + assert "minted" not in recorder # nothing minted on abort + assert "remote_name" not in recorder + + async def _async_value(value): return value