Skip to content

Every object download and upload returns 500 when the upstream key contains a dot (e.g. gitlab.com), though startup validation and batch calls accept it #56

Description

@matt-edmondson

What's wrong

ObjectStore.OpenStaging only accepts upstream keys made of [A-Za-z0-9_-] (GitLfsCache/Storage/ObjectStore.cs, IsValidUpstream / IsUpstreamCharacter), and throws ArgumentException otherwise. Nothing earlier applies the same rule:

  • GitLfsCacheOptionsValidator.ValidateUpstreams does not check the key's characters, so startup succeeds.
  • UpstreamRegistry and LfsRouteParser accept any key, so /…/{upstream}/…/info/lfs/objects/batch works and the batch is rewritten.

The failure only shows up on the object routes:

  • Download miss (Endpoints/ObjectRouteHandler.cs:332): the object is fetched from upstream, and then store.OpenStaging(route.Upstream) throws. The client gets a 500 and nothing is stored, so every download of every object fails, every time.
  • Upload (Endpoints/ObjectRouteHandler.cs:226): OpenStaging is the first thing called, so every PUT fails.

Failure scenario

Upstreams__gitlab.com__BaseUrl=https://gitlab.com
Repositories__0=**

git lfs pull through the cache: the batch call returns 200, then every object GET returns 500. A test using the repo's own ProxyFixture showed:

batch status 200
download threw ArgumentException: 'gitlab.com' is not a valid upstream key. (Parameter 'upstream')
upstream fetches: 1

A dotted key is a natural thing to configure, because upstreams are usually named after their host.

Suggested fix

Either:

  • make GitLfsCacheOptionsValidator reject keys that ObjectStore would reject, with a message naming the setting and the allowed characters, so the misconfiguration fails at startup rather than per request; or
  • map the upstream key to a safe directory name in ObjectStore (an escaped form or a hash), so any key the registry accepts can be stored.

Acceptance: a configuration with a dotted upstream key is either refused at startup with a clear error, or serves downloads and uploads successfully. There is a test for whichever option is chosen.

Activity

  1. matt-edmondson commented on Sep 28, 2026

    @matt-edmondson
    ContributorAuthor

    Decision (maintainer, 2026-09-28)

    • Map the upstream key to a safe directory name in ObjectStore. Use an escaped form or a hash, so any key the registry accepts can be stored. Dotted keys like gitlab.com are a natural configuration and must work.
    • Dotted keys are not rejected at startup.

    Next reader: implement the mapping in ObjectStore (OpenStaging and the lookup paths). It must be deterministic and collision-free, and any existing on-disk layout for currently valid keys must stay readable. Add a ProxyFixture test in which a gitlab.com upstream serves both a download and an upload successfully.


    Generated by Claude Code

  2. matt-edmondson commented on Sep 28, 2026

    @matt-edmondson
    ContributorAuthor

    Triage

    • Category: Bug
    • Priority: High. Naming an upstream after its host (gitlab.com) is the natural configuration, and with it every object download and upload returns 500. Startup and the batch call both succeed, so the misconfiguration surfaces only as per-request failures.
    • Area / suggested assignment: Storage, in GitLfsCache/Storage/ObjectStore.cs (IsValidUpstream / OpenStaging), with object-route tests in ProxyFixture
    • Duplicates: none open. It is closely related to When the staging file can't be created, pulls and pushes fail with 500 instead of bypassing the cache #57 (a staging-open failure turns into a 500); both surface at the same OpenStaging call sites in ObjectRouteHandler.
    • In progress: no matching open PR. Refuse to publish a staging file whose write failed #59 touches staging publish, not key validation.

    Notes: The maintainer decision is to map the key to a safe directory name that is deterministic, collision-free and backward-compatible with existing on-disk layouts, rather than rejecting dotted keys. Keep existing [A-Za-z0-9_-] keys mapping to themselves, so current caches stay readable without a migration.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions