Preserve numeric credential labels in auth removal
Resolve exact label matches before treating digit-only input as a positional index so destructive auth removal does not mis-target credentials named with numeric labels. Constraint: The CLI remove path must keep supporting existing index-based usage while adding safer label targeting Rejected: Ban numeric labels | labels are free-form and existing users may already rely on them Confidence: high Scope-risk: narrow Reversibility: clean Directive: When a destructive command accepts multiple identifier forms, prefer exact identity matches before fallback parsing heuristics Tested: Focused pytest slice for auth commands, credential pool recovery, and routing (273 passed); py_compile on changed Python files Not-tested: Full repository pytest suite
This commit is contained in:
@@ -760,11 +760,6 @@ class CredentialPool:
|
|||||||
raw = str(target or "").strip()
|
raw = str(target or "").strip()
|
||||||
if not raw:
|
if not raw:
|
||||||
return None, None, "No credential target provided."
|
return None, None, "No credential target provided."
|
||||||
if raw.isdigit():
|
|
||||||
index = int(raw)
|
|
||||||
if 1 <= index <= len(self._entries):
|
|
||||||
return index, self._entries[index - 1], None
|
|
||||||
return None, None, f"No credential #{index}."
|
|
||||||
|
|
||||||
for idx, entry in enumerate(self._entries, start=1):
|
for idx, entry in enumerate(self._entries, start=1):
|
||||||
if entry.id == raw:
|
if entry.id == raw:
|
||||||
@@ -779,6 +774,11 @@ class CredentialPool:
|
|||||||
return label_matches[0][0], label_matches[0][1], None
|
return label_matches[0][0], label_matches[0][1], None
|
||||||
if len(label_matches) > 1:
|
if len(label_matches) > 1:
|
||||||
return None, None, f'Ambiguous credential label "{raw}". Use the numeric index or entry id instead.'
|
return None, None, f'Ambiguous credential label "{raw}". Use the numeric index or entry id instead.'
|
||||||
|
if raw.isdigit():
|
||||||
|
index = int(raw)
|
||||||
|
if 1 <= index <= len(self._entries):
|
||||||
|
return index, self._entries[index - 1], None
|
||||||
|
return None, None, f"No credential #{index}."
|
||||||
return None, None, f'No credential matching "{raw}".'
|
return None, None, f'No credential matching "{raw}".'
|
||||||
|
|
||||||
def add_entry(self, entry: PooledCredential) -> PooledCredential:
|
def add_entry(self, entry: PooledCredential) -> PooledCredential:
|
||||||
|
|||||||
@@ -279,6 +279,56 @@ def test_auth_remove_accepts_label_target(tmp_path, monkeypatch):
|
|||||||
assert entries[0]["label"] == "work-account"
|
assert entries[0]["label"] == "work-account"
|
||||||
|
|
||||||
|
|
||||||
|
def test_auth_remove_prefers_exact_numeric_label_over_index(tmp_path, monkeypatch):
|
||||||
|
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
|
||||||
|
_write_auth_store(
|
||||||
|
tmp_path,
|
||||||
|
{
|
||||||
|
"version": 1,
|
||||||
|
"credential_pool": {
|
||||||
|
"openai-codex": [
|
||||||
|
{
|
||||||
|
"id": "cred-a",
|
||||||
|
"label": "first",
|
||||||
|
"auth_type": "oauth",
|
||||||
|
"priority": 0,
|
||||||
|
"source": "manual:device_code",
|
||||||
|
"access_token": "tok-a",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"id": "cred-b",
|
||||||
|
"label": "2",
|
||||||
|
"auth_type": "oauth",
|
||||||
|
"priority": 1,
|
||||||
|
"source": "manual:device_code",
|
||||||
|
"access_token": "tok-b",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"id": "cred-c",
|
||||||
|
"label": "third",
|
||||||
|
"auth_type": "oauth",
|
||||||
|
"priority": 2,
|
||||||
|
"source": "manual:device_code",
|
||||||
|
"access_token": "tok-c",
|
||||||
|
},
|
||||||
|
]
|
||||||
|
},
|
||||||
|
},
|
||||||
|
)
|
||||||
|
|
||||||
|
from hermes_cli.auth_commands import auth_remove_command
|
||||||
|
|
||||||
|
class _Args:
|
||||||
|
provider = "openai-codex"
|
||||||
|
target = "2"
|
||||||
|
|
||||||
|
auth_remove_command(_Args())
|
||||||
|
|
||||||
|
payload = json.loads((tmp_path / "hermes" / "auth.json").read_text())
|
||||||
|
labels = [entry["label"] for entry in payload["credential_pool"]["openai-codex"]]
|
||||||
|
assert labels == ["first", "third"]
|
||||||
|
|
||||||
|
|
||||||
def test_auth_reset_clears_provider_statuses(tmp_path, monkeypatch, capsys):
|
def test_auth_reset_clears_provider_statuses(tmp_path, monkeypatch, capsys):
|
||||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
|
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes"))
|
||||||
_write_auth_store(
|
_write_auth_store(
|
||||||
|
|||||||
Reference in New Issue
Block a user