diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b8299bd..61d45b62 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - Added an PNPM credential helper for Cloudsmith registries. `cloudsmith credential-helper install pnpm` installs an `pnpm-credential-cloudsmith` launcher binary and registers it in `~/.npmrc`, so npm authenticates to Cloudsmith registries automatically using your existing CLI credentials — no manual `npm login` required. Custom Cloudsmith registry domains are discovered via the API and cached locally; add extra hostnames with `--domain` (repeatable), disable discovery with `--no-discover`, or preview changes with `--dry-run`. Manage installed helpers with `cloudsmith credential-helper uninstall pnpm` and `cloudsmith credential-helper list`. +### Fixed + +- A failed package-file upload to the pre-signed storage URL (e.g. an expired upload token) now shows the real reason instead of just the generic HTTP status phrase (e.g. "Bad Request"). `upload_file()`/`multi_part_upload_file()` raised their `ApiException` without a `detail`, since that upload goes straight to the storage backend rather than through the Cloudsmith API client that normally populates it from the JSON error body; the detail is now extracted from the `` of the storage backend's XML error response. + ## [1.24.0] - 2026-08-18 ### Added diff --git a/NIX_NARINFO_CHECKSUM_SYNC_LOOP.md b/NIX_NARINFO_CHECKSUM_SYNC_LOOP.md new file mode 100644 index 00000000..87820dd8 --- /dev/null +++ b/NIX_NARINFO_CHECKSUM_SYNC_LOOP.md @@ -0,0 +1,173 @@ +# Nix packages loop forever on staging/production: `.narinfo` file checksum never computed + +## Summary + +A `nix` package push (`.nar.xz` primary file + `.narinfo` sidecar via +`--narinfo-file`) succeeds — both files upload, the package is created, both +files are correctly attached with checksums computed *client-side* — but the +package **never reaches a terminal sync state** on `cloudsmith.io` +(production) or `api-stg.cloudsmith.io` (staging). It cycles indefinitely +through sync stages, occasionally landing on a transient `Failed` before +being auto-requeued back to `Queued`, forever. `status_reason` stays `null` +throughout, so there is no self-serve diagnosis available. + +The same push, with the exact same two files, completes cleanly to `Fully +Synchronised` every time on a personal preview sandbox +(`kharrison.cloudsmith.sh`) — 3/3 clean runs. That environment split points +squarely at a backend difference in nix package ingestion, not the CLI, not +the input files, and not narinfo content. + +## Environments + +| Environment | Host | Result | +|---|---|---| +| Personal sandbox | `kharrison.cloudsmith.sh` | ✅ 3/3 clean, `Fully Synchronised` | +| Staging | `api-stg.cloudsmith.io` | ❌ Loops indefinitely, never terminal | +| Production | `api.cloudsmith.io` | ❌ Looped indefinitely until manually deleted | + +Client: `cloudsmith-cli` branch `kyleharrison/eng-12409/implement-nix-support`, +`cloudsmith-api` 2.0.31 (`--narinfo-file`, post-#365/#369 fixes — see "Why +this isn't a CLI bug" below). + +## The concrete, consistent lead: `.narinfo` file checksum is never populated + +In every failing run, the primary `.nar.xz` file's checksum is correctly +recorded, but the `.narinfo` sidecar file's checksum stays `null` even +though it's flagged `is_synchronised: true`: + +**Production** (`cloudsmith/nix-upstreams`, slug_perm `6GeDYe3c7Ogj`), from +the package-detail "delete" confirmation dump (captured directly from the +web UI while the package was still cycling): +``` +Package file: slug_perm="3dBhoCWaxZA3", filename="1asxnrnab7w7jz7ilcbnvn2m3m85c3sx9847ap9hyc9n4jsds04x.nar.xz", + checksum_sha256="9d00ddb42436310fd35587a0d4f56005d55185dd76311acf97879fa56cb65dab" +Package file: slug_perm="X2rsV7XGxJqZ", filename="vayna03pzyn98nlz8y4xcb1nncggb4nz.narinfo", + checksum_sha256="None" +``` + +**Staging** (`cloudsmith/testing-private`, slug_perm `0Zw8QNuCv0dr`), via +`cloudsmith list packages -q format:nix`: +``` +1asxnrnab7w7jz7ilcbnvn2m3m85c3sx9847ap9hyc9n4jsds04x.nar.xz checksum_sha256=9d00ddb4... is_synchronised=True +vayna03pzyn98nlz8y4xcb1nncggb4nz.narinfo checksum_sha256=None is_synchronised=True +``` + +**Sandbox control** (`cloudsmith/test-nix-support`, slug_perm +`Y3SexkLfLV3b`), same two source files, same command: +``` +1asxnrnab7w7jz7ilcbnvn2m3m85c3sx9847ap9hyc9n4jsds04x.nar.xz checksum_sha256=9d00ddb4... is_synchronised=True +vayna03pzyn98nlz8y4xcb1nncggb4nz.narinfo checksum_sha256=b23c05f7... is_synchronised=True +``` + +Same input files, same client, same command — the only variable is which +backend received the request. + +## Behaviour observed while stuck + +Polling `cloudsmith status` every 5–10s over several minutes on both +staging and production shows the package cycling through sync stages +without ever setting `ok` or `failed` to a stable `true`: + +``` +Queued → Retrieving Package File(s) → Enriching Package → +Parsing Package Metadata → Assembling/Verifying Package → Queued → ... +``` + +(Staging's cycle starts a stage earlier — `Adding Package to Repository` / +`Preparing for Synch` — before reaching the same later stages; likely just +queue-position noise on a busier shared environment, not a different bug.) + +On production, the web UI showed `Status: Enriching Package [resync] 3` — +i.e. an internal resync mechanism had already retried 3 times on its own, +independent of the CLI's `--sync-attempts` (which governs the CLI's own +post-`failed` resync calls, not whatever is looping this server-side). On +staging, a `list packages` snapshot caught the package as `stage="Adding +Package to Repository", status="Failed"`, but a `status` call moments later +showed it back at `"Queued"` — confirming it isn't just slow, it's actively +being requeued from a terminal-looking `Failed` state. + +**No self-remediation is possible while this is happening:** on production, +the package showed `is_deletable: false`, `is_resyncable: false`, +`is_cancellable: false` — all locked because `is_sync_in_flight: true`. The +only way to clear it was a manual deletion via the web UI's admin path, +which the account owner did directly (not via the CLI/API, which refused). + +## Why this isn't a CLI bug + +- Both files upload successfully and are correctly attached to the package + (visible in `files[]` with correct sizes and, for `.nar.xz`, correct + checksums) on every environment, including the two that loop. +- The narinfo-upload fix from + [#365](https://github.com/cloudsmith-io/cloudsmith-cli/pull/365) and the + `narinfo_file` rename (cloudsmith-api 2.0.31) are both confirmed working — + this is a completely different failure surface than the earlier 422/400 + upload issues. +- The CLI is not asked to compute or transmit the `.narinfo` file's + checksum as part of ingestion — that's a server-side computation during + sync, and it's specifically that computation that's missing on + staging/production but present on the sandbox. +- `status_reason` is `null` on every poll on every environment; the CLI is + faithfully surfacing every field the API returns, there's nothing being + dropped client-side. + +## Ruled out: client-side timing, ordering, and content collisions + +Given the divergence is so clean (sandbox always works, staging/production +never do), several client-controllable variables were tested directly +against the raw API (bypassing the CLI's own upload orchestration +entirely) to see if any of them mattered: + +| Variable tested | Where | Result | +|---|---|---| +| Same content pushed into a second repo (content-addressable-storage collision theory) | Sandbox | ✅ Clean, both repos fully synchronised, both checksums populated | +| Upload order reversed (`.nar.xz` before `.narinfo`) | Sandbox | ✅ Clean | +| Both files uploaded concurrently (threads) | Sandbox | ✅ Clean | +| `packages_upload_nix` fired before the S3 uploads finished (race) | Sandbox | ✅ Clean | +| Guaranteed-fresh, 100%-synthetic content (no real-world collision possible), normal CLI, normal timing | Staging | ❌ Still loops, `.narinfo` checksum still `None` | + +The synthetic-content test is the decisive one: a `.nar.xz`/`.narinfo` pair +generated from random bytes moments before the push (correct nix base32 +`FileHash`/`NarHash`/`StorePath` — it parses and creates the package fine) +cannot possibly have existed anywhere on staging before this test, ruling +out "this exact content already exists from a real nixpkgs mirror or +another team's test" as an explanation. It looped identically to the real +`xgcc` package. **This is not a race condition and not a content +collision — it's an unconditional backend behavior difference between the +sandbox and staging/production.** + +## Impact + +- Any `nix` package push to production or staging today gets stuck + indefinitely consuming worker/queue resources, with no terminal + success or failure state and no diagnosable reason. +- No API-driven cleanup is available while a package is stuck — it must be + removed out-of-band (as was done manually on production here). +- This blocks `nix` support from being usable on production entirely, not + just as a rough edge — every push will hit this. + +## Suggested next steps + +1. Diff whatever handles nix package ingestion between the + `kharrison.cloudsmith.sh` preview build and staging/production — the + narinfo-checksum computation step is the concrete divergence point. + Given the sandbox works and staging/production don't *unconditionally* + (confirmed with fresh synthetic content, normal timing — see "Ruled + out" above), this smells like a fix present on a feature branch/preview + deploy that hasn't been promoted, rather than a fresh regression, a + race, or a data-collision issue to hunt for from scratch. +2. Independent of the above: the "cycle back to `Queued` after transient + `Failed`" behavior and the total lack of `status_reason` deserve a fix + regardless of root cause — a stuck package should terminally fail with a + reason, not loop forever consuming resources. +3. Consider whether `is_deletable`/`is_resyncable`/`is_cancellable` should + allow intervention on a package that's been cycling for an abnormal + length of time, since `is_sync_in_flight` currently blocks all + self-service cleanup for exactly the packages that need it most. + +## Related + +See [`NIX_SCAN_BUG_REPORT.md`](./NIX_SCAN_BUG_REPORT.md) for an earlier, +now-resolved investigation on the sandbox (an expired S3 upload token, +unrelated to this) that surfaced the "Enriching Package" stage and +confirmed narinfo drives package metadata (`name`, `version`, `store_path`) +when ingestion succeeds. diff --git a/NIX_SCAN_BUG_REPORT.md b/NIX_SCAN_BUG_REPORT.md new file mode 100644 index 00000000..f8e36c9e --- /dev/null +++ b/NIX_SCAN_BUG_REPORT.md @@ -0,0 +1,288 @@ +# [RESOLVED] Nix packages never complete sync — fail at "Scanning Package" with no reason given + +## Resolution (2026-08-20) + +**Root cause:** an expired AWS STS session token used to sign pre-signed S3 +uploads for this sandbox's `cloudsmith-package-uploads-dev-kharrison` +bucket. Every file upload attempt — for *any* package, not just `nix` — +was failing at the S3 layer with `ExpiredToken` / "The provided token has +expired." A server reset on the sandbox refreshed the token, and a repeat +push completed a full sync: + +```json +"stage_str": "Fully Synchronised", +"status_str": "Completed", +"is_sync_completed": true, +"name": "xgcc", +"version": "15.2.0-libgcc", +"store_path": "/nix/store/vayna03pzyn98nlz8y4xcb1nncggb4nz-xgcc-15.2.0-libgcc" +``` + +This explains everything below without needing a nix-specific backend fix: +- The "Scanning Package" failures were a **downstream symptom**, not the + root cause — the scan stage was choking on packages whose files never + actually landed in S3 intact (the upload itself silently failed, but + package-create still succeeded since it only needs an upload + *identifier*, not confirmation the bytes arrived). +- A previously-unseen stage, **"Enriching Package"**, appears between + `Queued` and the scan/completion stages once uploads actually succeed — + this is what parses `.narinfo` content and populates `name`, `version`, + `store_path`, `fully_qualified_name` (all `null` in every failed attempt + above). So the original hypothesis that narinfo drives package metadata + was correct in spirit, even though it wasn't the cause of the failures + reported here. +- `security_scan_status` now resolves cleanly to `"Security Scanning Not + Supported"` (a real terminal state) instead of hanging at `"Awaiting + Security Scan"` — consistent with `is_security_scannable: false` being + intentional for `nix`, not a bug. + +**A real, separate CLI bug was found and fixed while debugging this:** +`upload_file()`/`multi_part_upload_file()` in +`cloudsmith_cli/core/api/files.py` raised `ApiException(status, headers=..., +body=...)` for a failed pre-signed upload **without a `detail=` kwarg**, so +`ApiException.__str__` fell back to the generic HTTP status phrase (e.g. +"Bad Request") and the CLI never showed the real reason — the `ExpiredToken` +message above only surfaced by manually reproducing the S3 request in +Python and reading the raw XML body. Fixed by extracting the `` +text from S3's XML error body (via a small regex, not a full XML parser — +this is response content from wherever `upload_url` points, so parsing it +with a real XML parser would add an XXE/entity-expansion attack surface for +no benefit over just pulling out one text field) and passing it through as +`ApiException.detail`. See `_s3_error_detail()` in +`cloudsmith_cli/core/api/files.py` and its tests in +`cloudsmith_cli/core/tests/test_files.py`. Future upload failures — token +expiry, bucket policy issues, whatever else — will now show their real +reason instead of a bare status phrase. + +**No nix-specific code change was needed or made** for the original +"Scanning Package" failure — it was pure sandbox infrastructure. The +investigation below is preserved as-is for context. + +**Update (2026-08-20, later the same day):** a distinct, more serious bug +was found reproducing on staging and production (not this sandbox) — nix +packages loop indefinitely rather than reaching a terminal state, with the +`.narinfo` file's checksum never getting computed server-side. See +[`NIX_NARINFO_CHECKSUM_SYNC_LOOP.md`](./NIX_NARINFO_CHECKSUM_SYNC_LOOP.md). +That issue is unrelated to the expired-token cause resolved here — it does +not reproduce on this sandbox at all. + +--- + +## Summary + +Every `nix` package pushed to a repository fails to synchronise. The package +is created successfully and its file(s) upload successfully, but the +package's sync pipeline dies at the **"Scanning Package"** stage +(`stage=4`) with `status="Failed"` and **`status_reason: null`** — no +diagnostic is surfaced anywhere in the API response. This reproduces +consistently (2/2 attempts), with or without the optional `narinfo` sidecar +file attached, and is not a timing/flake issue — both failed within 10–15 +seconds of upload. + +This blocks `nix` package support end-to-end: packages are visible in the +repo's package list, but are stuck in a failed, incomplete state +indefinitely (not downloadable, not synchronised) with no path to +resolution short of manual intervention. + +## Environment + +- **Instance:** `kharrison.cloudsmith.sh` (personal sandbox) +- **Organization/repo:** `cloudsmith/test-nix-support` +- **API version:** `cloudsmith-api` 2.0.30 (first release exposing + `NixPackageUploadRequest` / `NixUpstream*` models) +- **Client:** `cloudsmith-cli` branch `kyleharrison/eng-12409/implement-nix-support` + (commit `bf51ff3`), which adds `nix` to the CLI's supported push/upstream + formats — see [cloudsmith-cli#365](https://github.com/cloudsmith-io/cloudsmith-cli/pull/365) +- **Auth:** authenticated as `lskillen` (Lee Skillen) via sandbox routing + headers (`X-Region`, `X-Internal-User-Api-Key`) + +> **Update (cloudsmith-api 2.0.31):** the SDK's `narinfo` field was renamed +> to `narinfo_file` (CLI flag is now `--narinfo-file`), and the CLI's +> separate special-casing for the old name was dropped since it now matches +> the generic `_file`-suffix upload convention natively. That resolved a +> distinct CLI-side 422 (see "Why this isn't a CLI bug" below) but is +> unrelated to the "Scanning Package" failure this report is about — the +> commands below use the flag name current at the time of this report +> (`--narinfo`); substitute `--narinfo-file` if reproducing against +> 2.0.31+. + +## Steps to reproduce + +```bash +# 1. Push a nix package (.nar.xz) with its narinfo sidecar +cloudsmith push nix -W cloudsmith/test-nix-support \ + 1asxnrnab7w7jz7ilcbnvn2m3m85c3sx9847ap9hyc9n4jsds04x.nar.xz \ + --narinfo vayna03pzyn98nlz8y4xcb1nncggb4nz.narinfo + +# -> Package created OK: slug 1asxnrnab7w7jz7ilcbnvn2m3m85c3sx9847ap9hyc9n4-wr4r +# (slug_perm 685zgiIVJkip) + +# 2. Poll status +cloudsmith status cloudsmith/test-nix-support/685zgiIVJkip -F json +``` + +Repeated without `--narinfo` (package-file only) for comparison: + +```bash +cloudsmith push nix -W cloudsmith/test-nix-support \ + 1asxnrnab7w7jz7ilcbnvn2m3m85c3sx9847ap9hyc9n4jsds04x.nar.xz + +# -> Package created OK: slug 1asxnrnab7w7jz7ilcbnvn2m3m85c3sx9847ap9hyc9n4-iz3r +# (slug_perm wSHjxbhNfjjs) +``` + +Both were also cross-checked via `cloudsmith list packages cloudsmith/test-nix-support -F json`. + +## Expected result + +The package's sync pipeline completes (`is_sync_completed: true`), or, if it +genuinely cannot be scanned/synced, fails with a populated `status_reason` +explaining why. + +## Actual result + +Both packages permanently show: + +```json +"stage": 4, +"stage_str": "Scanning Package", +"status": 5, +"status_str": "Failed", +"status_reason": null, +"is_sync_completed": false, +"is_sync_failed": true, +"security_scan_status": "Awaiting Security Scan", +"is_security_scannable": false +``` + +Full package-list entries for both slugs (via `cloudsmith list packages`), trimmed to the relevant fields: + +
+slug_perm 685zgiIVJkip (pushed with --narinfo) + +```json +{ + "slug_perm": "685zgiIVJkip", + "format": "nix", + "files": [ + { + "filename": "1asxnrnab7w7jz7ilcbnvn2m3m85c3sx9847ap9hyc9n4jsds04x.nar.xz", + "tag": "pkg", + "is_primary": true, + "is_synchronised": true, + "size": 54524 + }, + { + "filename": "vayna03pzyn98nlz8y4xcb1nncggb4nz.narinfo", + "tag": "narinfo", + "is_primary": false, + "is_synchronised": false, + "size": 522 + } + ], + "num_files": 0, + "is_downloadable": false, + "is_security_scannable": false, + "security_scan_status": "Awaiting Security Scan", + "stage": 4, + "stage_str": "Scanning Package", + "status": 5, + "status_str": "Failed", + "status_reason": null, + "sync_progress": 45, + "uploaded_at": "2026-08-19T13:33:36.656111Z", + "sync_finished_at": "2026-08-19T13:33:47.005853Z" +} +``` +
+ +
+slug_perm wSHjxbhNfjjs (pushed without --narinfo) + +```json +{ + "slug_perm": "wSHjxbhNfjjs", + "format": "nix", + "files": [ + { + "filename": "1asxnrnab7w7jz7ilcbnvn2m3m85c3sx9847ap9hyc9n4jsds04x.nar.xz", + "tag": "pkg", + "is_primary": true, + "is_synchronised": true, + "size": 54524 + } + ], + "num_files": 0, + "is_downloadable": false, + "is_security_scannable": false, + "security_scan_status": "Awaiting Security Scan", + "stage": 4, + "stage_str": "Scanning Package", + "status": 5, + "status_str": "Failed", + "status_reason": null, + "sync_progress": 45, + "uploaded_at": "2026-08-19T13:36:03.758956Z", + "sync_finished_at": "2026-08-19T13:36:10.386008Z" +} +``` +
+ +## Notable inconsistency (possible root cause lead) + +`is_security_scannable: false` on both packages, yet the sync pipeline still +enters (and fails at) a "Scanning Package" stage, while +`security_scan_status` stays at its initial `"Awaiting Security Scan"` and +never progresses. This looks like a **generic malware/content-scan stage +that runs unconditionally for every format**, separate from the +format-gated vulnerability scanner. If that generic scan stage doesn't have +a handler for `.nar.xz` / `.narinfo` content yet, it would plausibly error +out immediately rather than skip — which matches the fast, deterministic +`Failed` we observed (~10–15s, well before any real scan could run) and the +complete absence of a `status_reason`. + +Also notable: on the package pushed with `--narinfo`, the primary `.nar.xz` +file reaches `is_synchronised: true`, but the `.narinfo` sidecar file stays +at `is_synchronised: false` — the package fails before ever finishing that +second file, regardless of it having uploaded correctly (checksums for both +files are present and correct in the response). + +## Why this isn't a CLI bug + +While investigating this, a real CLI bug was found and fixed separately +(`cloudsmith-cli` did not upload the `--narinfo` file via the file-upload +API before sending it to package-create, causing a 422 — fixed in +[#365](https://github.com/cloudsmith-io/cloudsmith-cli/pull/365)). After +that fix, `--narinfo` uploads correctly (see `is_synchronised: true` for the +primary file and a checksummed, attached `.narinfo` entry above) — but the +package still fails identically, **and pushing without `--narinfo` at all +fails the exact same way**. That rules the CLI out as the cause of the +scanning failure: the CLI is uploading files correctly and faithfully +surfacing every field the status API returns; there is no additional detail +being dropped client-side. + +## Impact + +- `nix` package support cannot ship — no package can reach a synchronised, + downloadable state on this environment. +- No self-serve diagnosis is possible from the CLI/API side: `status_reason` + is `null`, so whoever owns the scanning pipeline will need to check + service-side logs for these package/repo identifiers around the given + timestamps. + +## Suggested next steps (superseded — see Resolution above) + +These were written before the root cause was known; kept for context on +what was considered at the time. + +1. ~~Check the package-scanning worker's logs...~~ — not needed once the + expired token was identified as the actual cause. +2. ~~Confirm whether the generic "Scanning Package" stage has a content + handler for the `nix` format yet...~~ — `is_security_scannable: false` + for `nix` is confirmed intentional; it resolves cleanly to `"Security + Scanning Not Supported"` once uploads actually succeed. +3. Still worth doing, independent of this report: consider whether + `status_reason` should be populated for scan-stage failures in general, + so a future upload-layer issue (expired token or otherwise) is + diagnosable from `cloudsmith status` alone rather than requiring + backend log access. diff --git a/cloudsmith_cli/cli/commands/upstream.py b/cloudsmith_cli/cli/commands/upstream.py index df10effa..f999071d 100644 --- a/cloudsmith_cli/cli/commands/upstream.py +++ b/cloudsmith_cli/cli/commands/upstream.py @@ -31,6 +31,7 @@ "hex", "huggingface", "maven", + "nix", "npm", "nuget", "python", diff --git a/cloudsmith_cli/core/api/files.py b/cloudsmith_cli/core/api/files.py index 6c40ba5f..bd294cb5 100644 --- a/cloudsmith_cli/core/api/files.py +++ b/cloudsmith_cli/core/api/files.py @@ -1,6 +1,7 @@ """API - Files endpoints.""" import os +import re import click import cloudsmith_api @@ -14,6 +15,30 @@ from .init import get_api_client CHUNK_SIZE = 1024 * 1024 * 100 +#: Pulls the text out of an S3 XML error body, e.g. +#: ExpiredTokenThe provided token has +#: expired..... A plain regex rather than an XML parser: +#: this is untrusted response content (from wherever upload_url points), and +#: we only need one text field out of it, not general-purpose XML parsing +#: with its XXE/entity-expansion surface. +_S3_ERROR_MESSAGE_RE = re.compile(rb"(.*?)", re.DOTALL) + + +def _s3_error_detail(body): + """Extract the from an S3 XML error body, if there is one. + + upload_file()/multi_part_upload_file() PUT/POST straight to a pre-signed + S3 URL, not through the Cloudsmith API client, so their failures arrive + as S3's XML error format rather than the JSON body + catch_raise_api_exception() parses. Without this, ApiException.detail + stayed unset and the CLI only ever showed the generic HTTP status phrase + (e.g. "Bad Request") for failures like an expired upload token. + """ + if not body: + return None + + match = _S3_ERROR_MESSAGE_RE.search(body) + return match.group(1).decode("utf-8", errors="replace") if match else None def get_files_api(): @@ -90,7 +115,10 @@ def upload_file(upload_url, upload_fields, filepath, callback=None): resp.raise_for_status() except requests.RequestException as exc: raise ApiException( - resp.status_code, headers=exc.response.headers, body=exc.response.content + resp.status_code, + detail=_s3_error_detail(exc.response.content), + headers=exc.response.headers, + body=exc.response.content, ) @@ -117,6 +145,7 @@ def multi_part_upload_file( except requests.RequestException as exc: raise ApiException( resp.status_code, + detail=_s3_error_detail(exc.response.content), headers=exc.response.headers, body=exc.response.content, ) diff --git a/cloudsmith_cli/core/tests/test_files.py b/cloudsmith_cli/core/tests/test_files.py index 0750b78c..f4bbbfb5 100644 --- a/cloudsmith_cli/core/tests/test_files.py +++ b/cloudsmith_cli/core/tests/test_files.py @@ -2,7 +2,11 @@ from unittest.mock import Mock, patch -from ..api.files import multi_part_upload_file +import pytest +import requests + +from ..api.exceptions import ApiException +from ..api.files import _s3_error_detail, multi_part_upload_file from ..credentials.models import CredentialResult @@ -66,3 +70,58 @@ def test_sends_sso_token_as_bearer_authorization(self, tmp_path): headers = upload_and_capture_headers(tmp_path, credential) assert headers == {"Authorization": "Bearer sso-token"} + + +class TestS3ErrorDetail: + """A pre-signed S3 upload failure is an XML body, not the JSON + catch_raise_api_exception() parses, so ApiException.detail stayed unset + and the CLI could only ever show the generic HTTP status phrase. + """ + + def test_extracts_message_from_s3_error_xml(self): + body = ( + b'\n' + b"ExpiredToken" + b"The provided token has expired." + b"..." + ) + + assert _s3_error_detail(body) == "The provided token has expired." + + @pytest.mark.parametrize("body", [None, b"", b"not xml at all", b""]) + def test_returns_none_when_no_message_present(self, body): + assert _s3_error_detail(body) is None + + def test_multi_part_upload_failure_carries_s3_message_as_detail(self, tmp_path): + filepath = tmp_path / "package.raw" + filepath.write_bytes(b"payload") + + response = Mock( + status_code=400, + content=b"The provided token has expired.", + headers={}, + ) + response.raise_for_status.side_effect = requests.HTTPError(response=response) + + session = Mock() + session.put.return_value = response + + with ( + patch( + "cloudsmith_cli.core.api.files.create_requests_session", + return_value=session, + ), + patch("cloudsmith_cli.core.api.files.get_files_api"), + pytest.raises(ApiException) as exc_info, + ): + multi_part_upload_file( + Mock(credential=None), + upload_url="https://upload.example.invalid/parts", + owner="owner", + repo="repo", + filepath=str(filepath), + callback=lambda: None, + upload_id="upload-id", + ) + + assert exc_info.value.detail == "The provided token has expired." diff --git a/pyproject.toml b/pyproject.toml index 95548fe6..9b3cf99e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,7 +43,7 @@ dependencies = [ "click-didyoumean>=0.0.3", "click-spinner>=0.1.7", "json5>=0.9.0", # For parsing JSONC (JSON with comments) in VS Code settings - "cloudsmith-api>=2.0.29,<3.0", # Compatible upto (but excluding) 3.0+ + "cloudsmith-api>=2.0.31,<3.0", # Compatible up to (but excluding) 3.0+ "keyring>=25.4.1", "keyrings-alt>=5.0.2", "keyrings-cryptfile>=1.3.9", diff --git a/uv.lock b/uv.lock index 35e80bf3..688bdce1 100644 --- a/uv.lock +++ b/uv.lock @@ -507,7 +507,7 @@ wheels = [ [[package]] name = "cloudsmith-api" -version = "2.0.29" +version = "2.0.31" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "certifi" }, @@ -515,9 +515,9 @@ dependencies = [ { name = "six" }, { name = "urllib3" }, ] -sdist = { url = "https://files.pythonhosted.org/packages/17/72/9daab7a05f38c9bce89d51a94f1235cf2f8a641dd254eadb5f0a682a0d2a/cloudsmith_api-2.0.29.tar.gz", hash = "sha256:57771b2c7cfe681d685273dfcfd214dd36c2775d9fc080122b40b4d6c404c452", size = 667775, upload-time = "2026-07-15T13:33:51.588Z" } +sdist = { url = "https://files.pythonhosted.org/packages/3e/7c/c0e827e54989dc5031b95ceb6727997ea97b3bb5f22b9482dafa282f6798/cloudsmith_api-2.0.31.tar.gz", hash = "sha256:768888677ca6570dde5ea1b9b3254401786a1e4c2842c170ce715e1796306c91", size = 727667, upload-time = "2026-08-20T12:37:10.543Z" } wheels = [ - { url = "https://files.pythonhosted.org/packages/8f/7c/37f20207d34c42d0109786c504f0efe12328f8d64dc47f120eac5e90d1cb/cloudsmith_api-2.0.29-py2.py3-none-any.whl", hash = "sha256:db3cc74f5bfa96dd773c30251d681622405d5a0aa05bcead527e02cb9d744035", size = 1302309, upload-time = "2026-07-15T13:33:50.259Z" }, + { url = "https://files.pythonhosted.org/packages/6b/15/a5bcf3f2baeb403acb3dde53a7175bb9c06fc6d9844d324bf27f9f913cca/cloudsmith_api-2.0.31-py2.py3-none-any.whl", hash = "sha256:f6d8bd2889e5b58b06a81f2980911bae589631161fd2b4524cae346de0d72d4e", size = 1394274, upload-time = "2026-08-20T12:37:09.237Z" }, ] [[package]] @@ -583,7 +583,7 @@ requires-dist = [ { name = "click-configfile", specifier = ">=0.2.3" }, { name = "click-didyoumean", specifier = ">=0.0.3" }, { name = "click-spinner", specifier = ">=0.1.7" }, - { name = "cloudsmith-api", specifier = ">=2.0.29,<3.0" }, + { name = "cloudsmith-api", specifier = ">=2.0.31,<3.0" }, { name = "json5", specifier = ">=0.9.0" }, { name = "keyring", specifier = ">=25.4.1" }, { name = "keyrings-alt", specifier = ">=5.0.2" },