ghapi icon indicating copy to clipboard operation
ghapi copied to clipboard

GhApi.packages breaks when packages have a `/` in them

Open js-truework opened this issue 3 years ago • 2 comments

Note: Since I'm dealing with a private organization with private repositories and private containers, I've changed everything to myorg, myrepo, and mycontainer, respectively.

Summary

  • It appears that Ghapi is using urllib.parse quote on route items.
  • But the default behavior of urllib.parse.quote treats / as a safe character.
  • GHCR packages that belong to a particular repository have a / in their name and must be referenced by the API with an escaped / (i.e. %2F) (e.g. /orgs/myorg/packages/container/myrepo%2Fmycontainer is valid, but /orgs/myorg/packages/container/myrepo/mycontainer is a 404)
  • I am not sure that quote can be reliably called without treating / as a safe character, there may be other route options that need / to be safe? (I don't think so since we're dealing with url structure and users supplying / into any param is almost certainly not meant to be treated as a /, which would/could change the url structure)
  • I cannot pass in my own quoted string (e.g. myrepo%2Fmycontainer) because the % will get double encoded

Github Workflow

Unfortunately I do not have a public repository that I can share with you to reproduce this behavior. However, I can show you an example version of the github workflow that we use to build and subsequently tag and push the images which produces this behavior:

name: Example Build Workflow

on:
  push:
    branches:
      - master
  pull_request:
    branches: ["**"]

jobs:
  build-api:
    runs-on: ubuntu-latest
  env:
    image: api
    registry: ghcr.io
    IS_DEFAULT_BRANCH: ${{ github.ref == 'refs/heads/master' }}
    DOCKER_BUILDKIT: 1
  steps:
    - uses: actions/checkout@v2
    - uses: docker/login-action@v1
      with:
        registry: ${{ env.registry }}
        username: ${{ github.actor }}
        password: ${{ secrets.GITHUB_TOKEN }}
    - name: Docker Build (api)
       run: |
         docker build \
         --tag ${{env.image}} \
         --build-arg BUILDKIT_INLINE_CACHE=1 \
         --cache-from ubuntu:bionic \
         --cache-from ${{env.registry}}/${{github.repository}}/${{env.image}}:pr-${{github.event.pull_request.number}} \
         --cache-from ${{env.registry}}/${{github.repository}}/${{env.image}}:${{github.sha}} \
         --cache-from ${{env.registry}}/${{github.repository}}/${{env.image}}:latest \
         --file ./Dockerfile \
         .
    - name: Tag & Push (SHA)
       run: |
          docker tag ${{env.image}} ${{env.registry}}/${{github.repository}}/${{env.image}}:${{github.sha}}
          docker push ${{env.registry}}/${{github.repository}}/${{env.image}}:${{github.sha}}
    - name: Tag & Push (PR)
       if: ${{ ! fromJson(env.IS_DEFAULT_BRANCH) }}
       run: |
         docker tag ${{env.image}} ${{env.registry}}/${{github.repository}}/${{env.image}}:pr-${{github.event.pull_request.number}}
         docker push ${{env.registry}}/${{github.repository}}/${{env.image}}:pr-${{github.event.pull_request.number}}
    - name: Tag & Push (latest)
       if: ${{ fromJson(env.IS_DEFAULT_BRANCH) }}
       run: |
         docker tag ${{env.image}} ${{env.registry}}/${{github.repository}}/${{env.image}}:latest
         docker push ${{env.registry}}/${{github.repository}}/${{env.image}}:latest

Reproduce with Curl

I am also able to produce the expected behavior and the bad behavior with curl:

$ curl --head -H "Authorization: token $GITHUB_TOKEN" https://api.github.com/orgs/myorg/packages/container/myrepo%2Fmycontainer
HTTP/2 200 
server: GitHub.com
[...]
$ curl --head -H "Authorization: token $GITHUB_TOKEN" https://api.github.com/orgs/myorg/packages/container/myrepo/mycontainer  
HTTP/2 404 
server: GitHub.com

Print_summary Examples

import os
from ghapi.all import GhApi
from ghapi.core import print_summary

ORGANIZATION = "myorg"
PACKAGE_TYPE = "container"
REPO_NAME = "myrepo"
IMAGE_NAME = "mycontainer"
PACKAGE = f"{REPO_NAME}/{IMAGE_NAME}
API_KEY = os.environ.get("GITHUB_TOKEN")

api = GhApi(org=ORGANIZATION, package_type=PACKAGE_TYPE, token=API_KEY)

api.debug=print_summary
packages = api.packages.get_all_package_versions_for_package_owned_by_org(
    org=ORGANIZATION,
    package_type=PACKAGE_TYPE,
    package_name=PACKAGE
)
api.debug=None
$ python3 cleanup.py
{'data': None,
 'full_url': 'https://api.github.com/orgs/myorg/packages/container/myrepo%5C/mycontainer/versions',
 'headers': {'Accept': 'application/vnd.github.v3+json'},
 'method': 'GET'}
[...]
fastcore.basics.HTTP404NotFoundError: HTTP Error 404: Not Found

Same Script, urlencode the / myself

PACKAGE = f"{REPO_NAME}%2F{IMAGE_NAME}"
$ python3 cleanup.py 
{'data': None,
 'full_url': 'https://api.github.com/orgs/myorg/packages/container/myrepo%252Fmycontainer/versions',
 'headers': {'Accept': 'application/vnd.github.v3+json'},
 'method': 'GET'}
[...]
fastcore.basics.HTTP404NotFoundError: HTTP Error 404: Not Found

Other Variations I've tried

None of these work either, probably for pretty obvious reasons, but I wanted to show you the things I tried in as complete a detail as I can.

PACKAGE = r"myrepo/mycontainer"
[...]
PACKAGE = r"myrepo%2Fmycontainer"
[...]
PACKAGE = "myrepo\%2Fmycontainer"

Expected behavior

I should be able to call the packages api with packages that have / in their identifier, since github allows it and I can curl it.

Proposed Solutions

Option 1 - Blanket Change

if route:
    for k,v in route.items(): route[k] = quote(str(route[k]), safe='')
>>> import urllib.parse
>>> urllib.parse.quote("myrepo/mycontainer")
'myrepo/mycontainer'
>>> urllib.parse.quote("myrepo/mycontainer", safe='')
'myrepo%2Fmycontainer'

Option 2 - Skip Quote Config

if route: 
    if self.no_quote_routes:
        for k,v in route.items(): route[k] = str(route[k])
    else:
        for k,v in route.items(): route[k] = quote(str(route[k]))

Option 3 - No Safe Quote Config

if route:
    if self.no_safe_quote:
        for k,v in route.items(): route[k] = quote(str(route[k]), safe='')
    else: 
        for k,v in route.items(): route[k] = quote(str(route[k]))

js-truework avatar Aug 04 '21 04:08 js-truework

Was there every resolution to this? Did manually escaping the package name before using the API work?

stumpylog avatar Apr 27 '22 19:04 stumpylog

To my knowledge. this has not been addressed and nothing I was able to do fixed it locally without an upstream fix.

js-truework avatar Apr 27 '22 20:04 js-truework