Skip to content

Commit c67bcef

Browse files
Fix stale default configuration after removing a saved config
`__default__` could name a config that no longer exists, which suppressed the auto-default rule on the next save. - `load_config` drops a `__default__` naming no saved config; stale keys clear on the next write, since every mutation rebuilds the file from a load_config result. - Add `remove_named_config`, counterpart to `save_named_config`: one atomic write that deletes the config and re-points the default. Used by `logout` and `config -rm`. - Promote a lone survivor to default when the removed config held it, unless the environment configures Cloudinary - a stored default outranks the environment, so promoting would take precedence from CLOUDINARY_URL. `_is_sole_usable_config` holds that rule for the save and remove paths. - `cld logout` with a single saved login confirms it instead of showing a one-item menu. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent f903490 commit c67bcef

7 files changed

Lines changed: 212 additions & 39 deletions

File tree

‎cloudinary_cli/auth/__init__.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
from cloudinary_cli.defaults import logger, normalize_region, DEFAULT_REGION, CLOUDINARY_REGION
2424
from cloudinary_cli.utils.config_utils import (
2525
load_config,
26-
remove_config_keys,
26+
remove_named_config,
2727
save_named_config,
2828
is_reserved_config_name,
2929
)
@@ -78,7 +78,7 @@ def logout(name):
7878
return "not_oauth"
7979

8080
revoked = _revoke_login(name, saved[name])
81-
remove_config_keys(name)
81+
remove_named_config(name)
8282
return "removed" if revoked else "revoke_failed"
8383

8484

‎cloudinary_cli/core/auth.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,9 @@ def _select_oauth_login():
7474
logger.info("No saved OAuth logins to log out of.")
7575
return "none", None
7676

77+
if len(names) == 1:
78+
return _confirm_sole_login(names[0])
79+
7780
echo("Saved OAuth logins:")
7881
for i, name in enumerate(names, start=1):
7982
echo(f" {i}) {name}")
@@ -92,3 +95,16 @@ def _select_oauth_login():
9295
logger.error(f"Invalid selection '{choice}'. Expected a number between 1 and {len(names)}.")
9396
return "invalid", None
9497
return "selected", names[int(choice) - 1]
98+
99+
100+
def _confirm_sole_login(name):
101+
"""With a single saved login there is nothing to choose, so confirm that one rather than
102+
presenting a one-item menu. Logging out still revokes a token, so it is not done unasked."""
103+
choice = prompt_user(
104+
f"Log out of '{name}'? [y/N]: ",
105+
noninteractive_hint="Pass the configuration name directly: `cld logout <name>`.")
106+
if choice is None:
107+
return "invalid", None
108+
if choice.strip().lower() not in ("y", "yes"):
109+
return "cancelled", None
110+
return "selected", name

‎cloudinary_cli/core/config.py‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,10 @@
66
load_config,
77
verify_cloudinary_url,
88
save_named_config,
9-
remove_config_keys,
9+
remove_named_config,
1010
show_cloudinary_config,
1111
is_valid_cloudinary_config,
1212
user_config_names,
13-
get_default_config_name,
1413
set_default_config,
1514
clear_default_config,
1615
is_reserved_config_name,
@@ -101,12 +100,10 @@ def config_command(new, ls, as_json, show, rm, from_url, default, set_default, u
101100
clear_default_config()
102101
logger.info("Default configuration cleared.")
103102
elif rm:
104-
if remove_config_keys(rm):
105-
logger.warning(f"Configuration '{rm}' not found.")
106-
else:
107-
if get_default_config_name() == rm:
108-
clear_default_config()
103+
if remove_named_config(rm):
109104
logger.info(f"Configuration '{rm}' deleted.")
105+
else:
106+
logger.warning(f"Configuration '{rm}' not found.")
110107
elif ls:
111108
rows = list_configs()
112109
if as_json:

‎cloudinary_cli/defaults.py‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,10 @@
3737
# "warning:" prefix, so the copy-pasteable command lines stay clean.
3838
NO_CONFIG_MESSAGE = (
3939
"No Cloudinary configuration found.\n"
40-
" - Log in with OAuth: cld login\n"
41-
" - Add an API-key config: cld config -n <name> "
42-
"cloudinary://<api_key>:<api_secret>@<cloud_name> --set-default\n"
43-
" - Set an existing config\n"
44-
" as the default: cld config -d <name>\n"
45-
" - AI agents only, create\n"
46-
" an account for a human: cld agent signup <email> <framework> <model> <goal>"
40+
" - Log in with OAuth: cld login\n"
41+
" - Add an API-key config: cld config -n <name> cloudinary://<api_key>:<api_secret>@<cloud_name> --set-default\n"
42+
" - Set an existing config as the default: cld config -d <name>\n"
43+
" - AI agents - provision an environment to be claimed by a human: cld agent signup <email> <framework> <model> <goal>"
4744
)
4845

4946
# Shown when saved configs exist but none is active (no default set, no environment config, and no

‎cloudinary_cli/utils/config_utils.py‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,21 @@ def load_config():
7272
if stat is not None and stat == _config_cache_stat and _config_cache is not None:
7373
return dict(_config_cache) # copy: callers mutate the result in place (e.g. cfg.update(...))
7474
cfg = read_json_from_file(CLOUDINARY_CLI_CONFIG_FILE, does_not_exist_ok=True)
75+
_rectify_config(cfg)
7576
_config_cache, _config_cache_stat = cfg, stat
7677
return dict(cfg)
7778

7879

80+
def _rectify_config(cfg):
81+
"""Correct invalid state in a freshly-read config, in place, so no caller has to guard against
82+
it. The repair reaches disk on the next write, since every mutation rebuilds the file from a
83+
load_config result."""
84+
default = cfg.get(DEFAULT_CONFIG_KEY)
85+
if default is not None and default not in [k for k in cfg if k != DEFAULT_CONFIG_KEY]:
86+
del cfg[DEFAULT_CONFIG_KEY]
87+
logger.debug(f"Ignoring stored default '{default}': no such saved configuration.")
88+
89+
7990
def save_config(config):
8091
# 0600 from the start: the config file holds secrets (api_secret, account_url, OAuth tokens),
8192
# and writing the temp file 0600 before the atomic replace means it is never momentarily
@@ -140,13 +151,36 @@ def save_named_config(name, cloudinary_url, set_default=False):
140151
return "no"
141152

142153

154+
def remove_named_config(name):
155+
"""
156+
Delete a named configuration, and re-point the stored default when it named that config: a lone
157+
surviving config is promoted, otherwise the default is cleared. The counterpart to
158+
save_named_config: the single way to remove a saved config. Returns True if the configuration
159+
existed and was removed.
160+
"""
161+
with config_lock():
162+
cfg = load_config()
163+
if name not in user_config_names(cfg):
164+
return False
165+
del cfg[name]
166+
if cfg.get(DEFAULT_CONFIG_KEY) == name:
167+
del cfg[DEFAULT_CONFIG_KEY]
168+
remaining = user_config_names(cfg)
169+
if remaining and _is_sole_usable_config(remaining[0], cfg):
170+
cfg[DEFAULT_CONFIG_KEY] = remaining[0]
171+
save_config(cfg)
172+
return True
173+
174+
143175
def _should_auto_default(name):
144-
cfg = load_config()
145-
return (
146-
user_config_names(cfg) == [name]
147-
and not is_env_configured()
148-
and not get_default_config_name()
149-
)
176+
return _is_sole_usable_config(name) and not get_default_config_name()
177+
178+
179+
def _is_sole_usable_config(name, cfg=None):
180+
"""Whether name is the only config a bare `cld <command>` could use: the only saved config, with
181+
nothing configured in the environment. A stored default outranks the environment, so defaulting
182+
to a saved config while CLOUDINARY_URL is set would silently override the user's choice."""
183+
return user_config_names(cfg) == [name] and not is_env_configured()
150184

151185

152186
def user_config_names(cfg=None):

‎test/test_cli_config_oauth.py‎

Lines changed: 60 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import os
33
import time
44
import unittest
5+
from contextlib import contextmanager
56
from unittest.mock import patch
67

78
import cloudinary
@@ -11,6 +12,7 @@
1112
from test.oauth_helpers import jwt_access_token
1213
from cloudinary_cli.cli import cli
1314
from cloudinary_cli.utils.config_resolver import config_to_api_kwargs, get_cloudinary_config
15+
from cloudinary_cli.utils import config_utils
1416
from cloudinary_cli.utils.config_utils import config_to_dict, show_cloudinary_config
1517

1618

@@ -20,6 +22,27 @@ def _oauth_url(cloud="eu-cloud", region="api-eu"):
2022
expires_at=int(time.time()) + 300, region=region, issuer="https://oauth.cloudinary.com/"))
2123

2224

25+
@contextmanager
26+
def _patched_config_store(initial=None):
27+
"""Back load_config/save_config with an in-memory dict (no real config.json or lock)."""
28+
store = {"cfg": dict(initial or {})}
29+
30+
@contextmanager
31+
def _noop_lock():
32+
yield
33+
34+
def _load():
35+
cfg = dict(store["cfg"])
36+
config_utils._rectify_config(cfg) # the real load_config rectifies; mirror it here
37+
return cfg
38+
39+
with patch("cloudinary_cli.utils.config_utils.load_config", side_effect=_load), \
40+
patch("cloudinary_cli.utils.config_utils.save_config",
41+
side_effect=lambda cfg: store.__setitem__("cfg", dict(cfg))), \
42+
patch("cloudinary_cli.utils.config_utils.config_lock", _noop_lock):
43+
yield store
44+
45+
2346
class _RestoresSdkConfig(unittest.TestCase):
2447
def setUp(self):
2548
self._env_snapshot = dict(os.environ)
@@ -43,7 +66,7 @@ def test_removes_oauth_login(self):
4366
from cloudinary_cli.auth import logout
4467
saved = {"eu-cloud": _oauth_url()}
4568
with patch("cloudinary_cli.auth.load_config", return_value=saved), \
46-
patch("cloudinary_cli.auth.remove_config_keys") as remove, \
69+
patch("cloudinary_cli.auth.remove_named_config") as remove, \
4770
patch("cloudinary_cli.auth.flow.revoke") as revoke:
4871
self.assertEqual("removed", logout("eu-cloud"))
4972
remove.assert_called_once_with("eu-cloud")
@@ -54,7 +77,7 @@ def test_revoke_failure_still_removes_locally(self):
5477
from cloudinary_cli.auth import logout
5578
saved = {"eu-cloud": _oauth_url()}
5679
with patch("cloudinary_cli.auth.load_config", return_value=saved), \
57-
patch("cloudinary_cli.auth.remove_config_keys") as remove, \
80+
patch("cloudinary_cli.auth.remove_named_config") as remove, \
5881
patch("cloudinary_cli.auth.flow.revoke", side_effect=requests.ConnectionError()):
5982
self.assertEqual("revoke_failed", logout("eu-cloud"))
6083
remove.assert_called_once_with("eu-cloud") # local entry removed despite revoke failure
@@ -63,14 +86,14 @@ def test_refuses_non_oauth_config(self):
6386
from cloudinary_cli.auth import logout
6487
saved = {"mykey": "cloudinary://key:secret@cloud"}
6588
with patch("cloudinary_cli.auth.load_config", return_value=saved), \
66-
patch("cloudinary_cli.auth.remove_config_keys") as remove:
89+
patch("cloudinary_cli.auth.remove_named_config") as remove:
6790
self.assertEqual("not_oauth", logout("mykey"))
6891
remove.assert_not_called()
6992

7093
def test_missing_name(self):
7194
from cloudinary_cli.auth import logout
7295
with patch("cloudinary_cli.auth.load_config", return_value={}), \
73-
patch("cloudinary_cli.auth.remove_config_keys") as remove:
96+
patch("cloudinary_cli.auth.remove_named_config") as remove:
7497
self.assertEqual("not_found", logout("nope"))
7598
remove.assert_not_called()
7699

@@ -79,13 +102,15 @@ class TestLogoutInteractiveSelect(unittest.TestCase):
79102
"""`cld logout` with no name lists OAuth logins and removes the chosen one."""
80103

81104
runner = CliRunner()
105+
# The numbered menu only appears with more than one login, so menu tests need two.
106+
_TWO_LOGINS = {"cloud-a": _oauth_url("cloud-a"), "cloud-b": _oauth_url("cloud-b")}
82107

83108
def test_lists_only_oauth_and_removes_selected(self):
84109
saved = {"mykey": "cloudinary://key:secret@cloud",
85110
"cloud-a": _oauth_url("cloud-a"), "cloud-b": _oauth_url("cloud-b")}
86111
with patch("cloudinary_cli.auth.load_config", return_value=saved), \
87112
patch("cloudinary_cli.auth.refresh.load_config", return_value=saved), \
88-
patch("cloudinary_cli.auth.remove_config_keys") as remove, \
113+
patch("cloudinary_cli.auth.remove_named_config") as remove, \
89114
patch("cloudinary_cli.auth.flow.revoke"):
90115
result = self.runner.invoke(cli, ["logout"], input="2\n")
91116
self.assertIn("cloud-a", result.output)
@@ -96,40 +121,60 @@ def test_lists_only_oauth_and_removes_selected(self):
96121
def test_no_oauth_logins(self):
97122
with patch("cloudinary_cli.auth.refresh.load_config",
98123
return_value={"mykey": "cloudinary://key:secret@cloud"}), \
99-
patch("cloudinary_cli.auth.remove_config_keys") as remove:
124+
patch("cloudinary_cli.auth.remove_named_config") as remove:
100125
result = self.runner.invoke(cli, ["logout"], input="\n")
101126
self.assertIn("No saved OAuth logins", result.output)
102127
remove.assert_not_called()
103128

104129
def test_cancel_on_empty_input(self):
105130
with patch("cloudinary_cli.auth.refresh.load_config", return_value={"cloud-a": _oauth_url("cloud-a")}), \
106-
patch("cloudinary_cli.auth.remove_config_keys") as remove:
131+
patch("cloudinary_cli.auth.remove_named_config") as remove:
107132
result = self.runner.invoke(cli, ["logout"], input="\n")
108133
remove.assert_not_called()
109134
self.assertEqual(0, result.exit_code)
110135

111136
def test_invalid_non_numeric_errors(self):
112-
with patch("cloudinary_cli.auth.refresh.load_config", return_value={"cloud-a": _oauth_url("cloud-a")}), \
113-
patch("cloudinary_cli.auth.remove_config_keys") as remove:
137+
with patch("cloudinary_cli.auth.refresh.load_config", return_value=self._TWO_LOGINS), \
138+
patch("cloudinary_cli.auth.remove_named_config") as remove:
114139
result = self.runner.invoke(cli, ["logout"], input="sdfdsf\n", standalone_mode=False)
115140
self.assertIn("Invalid selection", result.output)
116141
self.assertFalse(result.return_value) # main() maps falsy -> exit 1
117142
remove.assert_not_called()
118143

119144
def test_out_of_range_errors(self):
120-
with patch("cloudinary_cli.auth.refresh.load_config", return_value={"cloud-a": _oauth_url("cloud-a")}), \
121-
patch("cloudinary_cli.auth.remove_config_keys") as remove:
145+
with patch("cloudinary_cli.auth.refresh.load_config", return_value=self._TWO_LOGINS), \
146+
patch("cloudinary_cli.auth.remove_named_config") as remove:
122147
result = self.runner.invoke(cli, ["logout"], input="5\n", standalone_mode=False)
123148
self.assertIn("Invalid selection", result.output)
124149
self.assertFalse(result.return_value)
125150
remove.assert_not_called()
126151

152+
def test_sole_login_confirms_instead_of_listing(self):
153+
saved = {"cloud-a": _oauth_url("cloud-a")}
154+
with patch("cloudinary_cli.auth.load_config", return_value=saved), \
155+
patch("cloudinary_cli.auth.refresh.load_config", return_value=saved), \
156+
patch("cloudinary_cli.auth.remove_named_config") as remove, \
157+
patch("cloudinary_cli.auth.flow.revoke"):
158+
result = self.runner.invoke(cli, ["logout"], input="y\n")
159+
self.assertNotIn("Saved OAuth logins:", result.output) # no one-item menu
160+
self.assertIn("cloud-a", result.output)
161+
remove.assert_called_once_with("cloud-a")
162+
163+
def test_sole_login_declined_removes_nothing(self):
164+
saved = {"cloud-a": _oauth_url("cloud-a")}
165+
with patch("cloudinary_cli.auth.load_config", return_value=saved), \
166+
patch("cloudinary_cli.auth.refresh.load_config", return_value=saved), \
167+
patch("cloudinary_cli.auth.remove_named_config") as remove:
168+
result = self.runner.invoke(cli, ["logout"], input="n\n")
169+
remove.assert_not_called()
170+
self.assertEqual(0, result.exit_code)
171+
127172
def test_noninteractive_stdin_errors_with_hint(self):
128173
# Closed stdin (no input at all): the selection cannot be made, so error with the
129174
# non-interactive form (`cld logout <name>`) and exit non-zero, not a silent no-op.
130175
import builtins
131176
with patch("cloudinary_cli.auth.refresh.load_config", return_value={"cloud-a": _oauth_url("cloud-a")}), \
132-
patch("cloudinary_cli.auth.remove_config_keys") as remove, \
177+
patch("cloudinary_cli.auth.remove_named_config") as remove, \
133178
patch.object(builtins, "input", side_effect=EOFError()):
134179
result = self.runner.invoke(cli, ["logout"], standalone_mode=False)
135180
self.assertIn("cld logout <name>", result.output)
@@ -811,12 +856,11 @@ def test_synthetic_row_name_parenthesized_in_table_and_json(self):
811856
self.assertIn("(environment)", by_name)
812857

813858
def test_rm_of_default_clears_it(self):
814-
with patch("cloudinary_cli.core.config.remove_config_keys", return_value=[]), \
815-
patch("cloudinary_cli.core.config.get_default_config_name", return_value="prod"), \
816-
patch("cloudinary_cli.core.config.clear_default_config") as clear:
859+
stored = {"prod": "cloudinary://k:s@prod", "__default__": "prod"}
860+
with _patched_config_store(stored) as store:
817861
result = self.runner.invoke(cli, ['config', '-rm', 'prod'])
818862
self.assertEqual(0, result.exit_code, result.output)
819-
clear.assert_called_once()
863+
self.assertEqual({}, store["cfg"]) # the name and the default it held both gone
820864

821865
def test_reserved_name_rejected_on_new(self):
822866
result = self.runner.invoke(

0 commit comments

Comments
 (0)