Skip to content

fix(ENG-13605): authenticate multi-part uploads via the credential chain - #327

Merged
cloudsmith-iduffy merged 2 commits into
masterfrom
eng-13605-raw-packages-100-mb-fail-to-upload-via-cli-multipart
Jul 30, 2026
Merged

fix(ENG-13605): authenticate multi-part uploads via the credential chain#327
cloudsmith-iduffy merged 2 commits into
masterfrom
eng-13605-raw-packages-100-mb-fail-to-upload-via-cli-multipart

Conversation

@cloudsmith-iduffy

@cloudsmith-iduffy cloudsmith-iduffy commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Fixes ENG-13605.

Summary

Raw packages over 100MB failed to upload with 404 - Not Found. It's an authentication bug, not a size or CDN one — the size threshold only determines whether the multi-part code path runs at all.

Root cause

multi_part_upload_file is one of the few places in the CLI making a raw HTTP call outside the swagger client, and it built its auth header from the wrong source:

headers = {"X-Api-Key": opts.api_key}

opts.api_key is a legacy field populated by only three of the five credential sources. Everything resolved by CredentialProviderChain lands on opts.credential instead:

Credential source Populates opts.api_key? Multi-part upload
--api-key flag yes worked
CLOUDSMITH_API_KEY yes worked
credentials.ini yes worked
OIDC auto-discovery no 404
SSO / keyring no 404

Three things then compound into a misleading error:

  1. requests silently drops headers whose value is None — the PUT goes out with no auth at all.
  2. upload.cloudsmith.io runs the standard DRF auth chain, and an anonymous request there returns 404, not 401.
  3. Every preceding API call uses the swagger client, which is correctly authenticated — so the failure appears only at the byte upload.

The CLI then prints Hint: This usually means the user/org is wrong or not visible., pointing at entirely the wrong cause.

This also explains the two details that made the issue look server-side: other formats work because non-multipart uploads use a presigned POST straight to S3 needing no Cloudsmith auth header, and the <=100MB path never touches this code.

The fix

The credential-to-header mapping lives once, on CredentialResult.auth_headers():

auth_type Header
api_key X-Api-Key: <token>
bearer Authorization: Bearer <token>

The two are not interchangeable, which is why the mapping is explicit rather than always using X-Api-Key. Measured against production with a real bearer token: Authorization: Bearer → 200 authenticated; the same token raw in X-Api-Key → 401 (ApiKeyHeaderAuthentication aborts the chain with force=True); prefixed with Bearer inside X-Api-Key → 200 but authenticated as anonymous. Tokens with auth_type="api_key" authenticate correctly via X-Api-Key regardless of form.

multi_part_upload_file now reads opts.credential and nothing else. An earlier revision fell back to opts.api_key when no credential resolved, and also raised an explicit error in that case; both were dropped as dead code — every source that populates opts.api_key also feeds the chain, so no credential means no credentials at all, and the push already fails at the first API call (Checking raw package upload parameters ... ERROR) long before the multi-part code runs.

The same defect, one command over

cloudsmith download resolved auth from opts.api_key plus its own direct keyring read. Between them those cover neither OIDC auto-discovery nor any future credential source, so downloading from a private repository in an OIDC-authenticated pipeline was attempted anonymously. It now uses the same resolved credential as everything else.

The 401 hint had the same shape of bug, and it is what made this issue hard to diagnose in the first place: branching on opts.api_key plus a keyring read meant an OIDC-authenticated session was told "You don't have an API key or access token set" while authenticated, and an SSO session with an expired token could be told to check its permissions instead of to re-run cloudsmith auth. It now branches on opts.credential.

opts.api_key is no longer a credential

It is now read in exactly one place — cli/decorators.py:125, where credentials.ini feeds the chain. The chain alone decides what authenticates a request. Two consequences beyond the call sites above:

  • new_config_messaging compares against opts.api_key_from_file, which is what its "your default credentials config file" wording actually means; opts.api_key may by then hold a flag or env key instead. Behaviour is unchanged in practice — both fields hold the same value for all four tokens.py commands.
  • The opts.api_key = api_key write in common_api_auth_options, whose comment said it was kept "for any code that still reads it directly", is deleted. It has no readers left. It was a write after line 125, so removing it cannot affect the chain's file input.

Both direct keyring.get_access_token() reads that bypassed the chain are gone with it.

core/api/init.py is deliberately untouched. It doesn't write a flat header dict — it routes into the swagger client's two separate auth slots (config.headers["Authorization"] vs config.api_key["X-Api-Key"], the latter read back by metadata.py and unset_api_key) — so putting it through auth_headers() would mean branching on the returned header name to pick a slot, which is worse than the branch it replaces.

Verification

Against production, 150MB raw pushes, before and after, covering both previously-broken sources:

Path Before After
api_key credential with opts.api_key unset ApiException status: 404 -> Not Found 2 parts uploaded, upload completed
Chain-resolved credential → iduffy-demo/default reported 404 verbatim Created: .../sso-testbin-15c6
Chain-resolved credential → cloudsmith/iduffy-scratchpad reported 404 verbatim Created: .../sso-scratchpadbin-47w4

Successful uploads verified byte-for-byte: 157286400 bytes, md5 3d3be108b6b902c41404da7adff4a8da, status Completed. The previously-working env-var path was re-tested and is unaffected.

Pre-fix output reproduced the customer report exactly:

Uploading sso-scratchpad.bin:
ERROR
Failed to upload file! (status: 404 - Not Found)
Hint: This usually means the user/org is wrong or not visible.

The upload tests assert the headers multi_part_upload_file actually puts on the wire, so they fail against the original bug rather than against a since-deleted helper. Full suite 558 passed, 40 skipped; pre-commit clean.

Note for reviewers: why CI never caught this

The existing 300MiB multipart integration test (test_push_and_delete_raw_package[314572800]) passes and always has. CI authenticates with CLOUDSMITH_API_KEY as an env var — one of the three sources that populates opts.api_key — so the test only ever exercises a configuration in which this bug cannot occur. (CI also targets staging rather than production, though that turned out not to be the deciding factor.)

Left as-is deliberately for this PR, but worth a follow-up: coverage across the credential sources would have caught this, and would have caught the cloudsmith download case too.

🤖 Generated with Claude Code

@cloudsmith-iduffy
cloudsmith-iduffy force-pushed the eng-13605-raw-packages-100-mb-fail-to-upload-via-cli-multipart branch 2 times, most recently from 7b4c8ac to fa8394c Compare July 30, 2026 13:48
Raw packages over 100MB failed to upload with "404 - Not Found" whenever the
session was authenticated by SSO or OIDC rather than an API key.

multi_part_upload_file built its auth header from opts.api_key, which is only
populated by --api-key, CLOUDSMITH_API_KEY or credentials.ini. Credentials
resolved by the provider chain land on opts.credential instead, so with an SSO
session or OIDC auto-discovery opts.api_key was None. requests silently drops
None-valued headers, so the part upload went out unauthenticated, and the
upload endpoint answers 404 (not 401) for an anonymous request. Every
preceding API call succeeded, so the failure surfaced only at the byte upload
with a hint pointing at the wrong cause.

Map credential type to header once, on CredentialResult.auth_headers(): SSO
access tokens as a bearer Authorization header (the only header the SSO
authenticators read -- a raw JWT in X-Api-Key is rejected outright, and a
Bearer-prefixed one authenticates as anonymous), API keys and OIDC tokens as
X-Api-Key. Both raw HTTP callers use it.

cloudsmith download had the same defect, one command over: it resolved auth
from opts.api_key plus its own keyring read, which between them cover neither
OIDC auto-discovery nor any future source, so downloads from a private
repository in an OIDC pipeline were attempted anonymously.

opts.api_key is now read in exactly one place, cli/decorators.py:125, where
credentials.ini feeds the chain. The chain alone decides what authenticates a
request:

  - the multi-part upload and download read opts.credential
  - the 401 hint branches on opts.credential rather than opts.api_key plus its
    own keyring read, so an OIDC session is no longer told it has no
    credentials set, and an expired SSO token is no longer reported as a
    permissions problem -- the wrong hint is what made this issue hard to
    diagnose in the first place
  - new_config_messaging compares against opts.api_key_from_file, which is
    what its "your default credentials config file" wording means
  - the "keep opts.api_key populated for any code that still reads it
    directly" write is dropped, having no readers left

Both direct keyring reads that bypassed the chain are gone with it.

core/api/init.py is deliberately untouched: it routes into the swagger
client's two separate auth slots rather than writing a flat header dict, so
putting it through auth_headers() would mean branching on the returned header
name to pick a slot.

Verified against production for both paths: an OIDC-style api_key credential,
and a real SSO session pushing 150MB to cloudsmith/iduffy-scratchpad and
iduffy-demo/default. Uploads complete with matching size and MD5; before the
change both reproduced the reported 404 verbatim. The upload tests assert the
headers multi_part_upload_file puts on the wire, so they fail against the
original bug rather than against a helper.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@cloudsmith-iduffy
cloudsmith-iduffy force-pushed the eng-13605-raw-packages-100-mb-fail-to-upload-via-cli-multipart branch from fa8394c to b54cf7e Compare July 30, 2026 13:53
@cloudsmith-iduffy
cloudsmith-iduffy marked this pull request as ready for review July 30, 2026 13:57
@cloudsmith-iduffy
cloudsmith-iduffy requested a review from a team as a code owner July 30, 2026 13:57
Copilot AI review requested due to automatic review settings July 30, 2026 13:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes an authentication gap where certain raw HTTP code paths (notably multi-part uploads and cloudsmith download) bypassed the resolved credential chain, causing anonymous requests (and misleading 404/401 behavior) when using SSO/keyring or OIDC-derived credentials.

Changes:

  • Multi-part raw uploads now authenticate using the resolved credential (opts.credential) and its correct header mapping (X-Api-Key vs Authorization: Bearer).
  • cloudsmith download and the CLI’s 401 hint logic now rely on the resolved credential chain rather than opts.api_key and direct keyring reads.
  • Adds targeted tests for multi-part upload headers, download auth resolution, config messaging, and 401 hint behavior; updates the changelog.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
cloudsmith_cli/core/api/files.py Uses the resolved credential chain to authenticate multi-part PUT part uploads.
cloudsmith_cli/core/credentials/models.py Centralizes credential → raw HTTP auth header mapping via CredentialResult.auth_headers().
cloudsmith_cli/core/download.py Resolves download authentication from the resolved credential chain (with --api-key override).
cloudsmith_cli/core/config.py Updates config messaging to compare against the file-sourced key (api_key_from_file).
cloudsmith_cli/cli/exceptions.py Updates 401 hints to reflect the resolved credential rather than legacy opts.api_key/keyring reads.
cloudsmith_cli/cli/decorators.py Removes the legacy opts.api_key backfill to reduce direct-field reliance.
cloudsmith_cli/core/tests/test_files.py Verifies multi-part upload sends the correct auth headers for API key vs bearer credentials.
cloudsmith_cli/core/tests/test_download.py Updates/extends auth resolution tests to cover credential-chain API key, SSO bearer, overrides, and none.
cloudsmith_cli/core/tests/test_config.py Adds tests validating config messaging compares against the credentials-file key.
cloudsmith_cli/cli/tests/test_exceptions.py Adds tests ensuring 401 hints branch on resolved credential type/presence.
CHANGELOG.md Documents the multipart upload, download auth, and 401 hint fixes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread CHANGELOG.md Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@cloudsmith-iduffy
cloudsmith-iduffy merged commit ae0b5ff into master Jul 30, 2026
26 checks passed
@cloudsmith-iduffy
cloudsmith-iduffy deleted the eng-13605-raw-packages-100-mb-fail-to-upload-via-cli-multipart branch July 30, 2026 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants