Skip to content

Commit 621f913

Browse files
authored
fix: harden clob download error handling (#235)
## Summary - keep default blob/clob downloads on safe relative paths when IDs start with `/` or `\` - fix CLOB download/list scoped-read hint handling so HTTP failures report the intended access hint - avoid showing CDA credential-scope hints for local filesystem write/path failures - log the actual blob download path after extension detection ## Why Blob and CLOB IDs can look like paths, but default downloads should never write to filesystem root. Local destination failures should point users toward `--dest`, while real CDA HTTP failures can still show credential-scope guidance. ## Validation - `poetry run pytest tests/commands/test_download_dest_safety.py tests/commands/test_blob_upload.py tests/commands/test_clob.py -q` - `poetry run pytest -q` - `poetry check` (warnings only for existing Poetry metadata deprecations)
1 parent 1a3b26d commit 621f913

4 files changed

Lines changed: 72 additions & 10 deletions

File tree

cwmscli/commands/blob.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -613,12 +613,12 @@ def download_cmd(
613613
try:
614614
blob_content = cwms.get_blob(office_id=office, blob_id=bid)
615615
target = dest or _default_download_dest(bid)
616-
_save_blob_content(
616+
saved_target = _save_blob_content(
617617
blob_content,
618618
dest=target,
619619
media_type_hint=_blob_media_type(cwms, office, bid),
620620
)
621-
logging.info(f"Downloaded blob to: {target}")
621+
logging.info(f"Downloaded blob to: {saved_target}")
622622
except requests.HTTPError as e:
623623
detail = getattr(e.response, "text", "") or str(e)
624624
logging.error(f"Failed to download (HTTP): {detail}")
@@ -632,6 +632,7 @@ def download_cmd(
632632
sys.exit(1)
633633
except Exception as e:
634634
logging.error(format_local_download_error(e, BLOB_DOCS_URL))
635+
# Local write/path failures are not CDA credential scope problems.
635636
if not isinstance(e, (OSError, ValueError)):
636637
log_scoped_read_hint(
637638
credential_kind=credential_kind,

cwmscli/commands/clob.py

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,15 @@
1212
from cwmscli.utils import (
1313
format_local_download_error,
1414
get_api_key,
15+
get_saved_login_token,
1516
has_invalid_chars,
17+
init_cwms_session,
1618
log_scoped_read_hint,
1719
validate_default_download_dest,
1820
)
21+
from cwmscli.utils.click_help import DOCS_BASE_URL
22+
23+
CLOB_DOCS_URL = f"{DOCS_BASE_URL}/cli/clob.html"
1924

2025

2126
def _join_api_url(api_root: str, path: str) -> str:
@@ -28,6 +33,16 @@ def _resolve_optional_api_key(api_key: Optional[str], anonymous: bool) -> Option
2833
return get_api_key(api_key, None)
2934

3035

36+
def _resolve_credential_kind(api_key: Optional[str], anonymous: bool) -> Optional[str]:
37+
if anonymous:
38+
return None
39+
if get_saved_login_token():
40+
return "token"
41+
if _resolve_optional_api_key(api_key, anonymous):
42+
return "api_key"
43+
return None
44+
45+
3146
def _write_clob_content(content: str, dest: str) -> str:
3247
os.makedirs(os.path.dirname(dest) or ".", exist_ok=True)
3348
with open(dest, "w", encoding="utf-8", newline="") as f:
@@ -36,12 +51,17 @@ def _write_clob_content(content: str, dest: str) -> str:
3651

3752

3853
def _default_download_dest(clob_id: str) -> str:
39-
return validate_default_download_dest(clob_id, resource_name="Clob")
54+
return validate_default_download_dest(
55+
clob_id,
56+
resource_name="Clob",
57+
docs_url=CLOB_DOCS_URL,
58+
)
4059

4160

4261
def _clob_endpoint_id(clob_id: str) -> tuple[str, Optional[str]]:
4362
normalized = clob_id.upper()
4463
if has_invalid_chars(normalized):
64+
# Path-like IDs use the placeholder route and keep the real ID in query params.
4565
return "ignored", normalized
4666
return normalized, None
4767

@@ -190,8 +210,8 @@ def download_cmd(
190210
f"DRY RUN: would GET {api_root} clob with clob-id={clob_id} office={office}."
191211
)
192212
return
193-
resolved_api_key = _resolve_optional_api_key(api_key, anonymous)
194-
cwms.init_session(api_root=api_root, api_key=resolved_api_key)
213+
credential_kind = _resolve_credential_kind(api_key, anonymous)
214+
init_cwms_session(cwms, api_root=api_root, api_key=api_key, anonymous=anonymous)
195215
bid = clob_id.upper()
196216
logging.debug(f"Office={office} clobID={bid}")
197217

@@ -215,15 +235,15 @@ def download_cmd(
215235
detail = getattr(e.response, "text", "") or str(e)
216236
logging.error(f"Failed to download (HTTP): {detail}")
217237
log_scoped_read_hint(
218-
api_key=resolved_api_key,
238+
credential_kind=credential_kind,
219239
anonymous=anonymous,
220240
office=office,
221241
action="download",
222242
resource="clob content",
223243
)
224244
sys.exit(1)
225245
except Exception as e:
226-
logging.error(format_local_download_error(e, ""))
246+
logging.error(format_local_download_error(e, CLOB_DOCS_URL))
227247
sys.exit(1)
228248

229249

@@ -305,8 +325,8 @@ def list_cmd(
305325
api_key: str,
306326
anonymous: bool = False,
307327
):
308-
resolved_api_key = _resolve_optional_api_key(api_key, anonymous)
309-
cwms.init_session(api_root=api_root, api_key=resolved_api_key)
328+
credential_kind = _resolve_credential_kind(api_key, anonymous)
329+
init_cwms_session(cwms, api_root=api_root, api_key=api_key, anonymous=anonymous)
310330
try:
311331
df = list_clobs(
312332
office=office,
@@ -319,7 +339,7 @@ def list_cmd(
319339
)
320340
except Exception:
321341
log_scoped_read_hint(
322-
api_key=resolved_api_key,
342+
credential_kind=credential_kind,
323343
anonymous=anonymous,
324344
office=office,
325345
action="list",

cwmscli/utils/__init__.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,7 @@ def validate_default_download_dest(
270270
f"Pass --dest explicitly if needed."
271271
)
272272

273+
# Leading separators can be part of a CDA ID; default downloads stay relative.
273274
target = raw_id.lstrip("/\\")
274275
if not target:
275276
message = (

tests/commands/test_clob.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,46 @@ class FakeHTTPError(Exception):
307307
assert "/cli/blob.html" not in caplog.text
308308

309309

310+
def test_download_cmd_http_error_logs_scope_hint(
311+
tmp_path, monkeypatch: pytest.MonkeyPatch, caplog
312+
):
313+
class FakeResponse:
314+
text = "Forbidden"
315+
316+
class FakeHTTPError(Exception):
317+
response = FakeResponse()
318+
319+
class FakeCwms:
320+
@staticmethod
321+
def init_session(api_root, api_key):
322+
return None
323+
324+
@staticmethod
325+
def get_clob(office_id, clob_id):
326+
raise FakeHTTPError()
327+
328+
monkeypatch.setitem(sys.modules, "cwms", FakeCwms)
329+
monkeypatch.setattr("cwmscli.commands.clob.cwms", FakeCwms)
330+
monkeypatch.setattr(
331+
"cwmscli.commands.clob.requests",
332+
types.SimpleNamespace(HTTPError=FakeHTTPError),
333+
)
334+
335+
with caplog.at_level(logging.WARNING), pytest.raises(SystemExit) as exc:
336+
download_cmd(
337+
clob_id="test_clob",
338+
dest=str(tmp_path / "downloaded.txt"),
339+
office="SWT",
340+
api_root="https://example.test/",
341+
api_key="apikey 123",
342+
dry_run=False,
343+
)
344+
345+
assert exc.value.code == 1
346+
assert "Access scope hint: an API key was sent" in caplog.text
347+
assert "clob content" in caplog.text
348+
349+
310350
def test_list_cmd_initializes_session_with_api_key(monkeypatch: pytest.MonkeyPatch):
311351
calls = []
312352

0 commit comments

Comments
 (0)