Skip to content

Report a malformed github release slug instead of panicking - #461

Open
arpitjain099 wants to merge 1 commit into
carvel-dev:developfrom
arpitjain099:fix/github-release-slug-format
Open

Report a malformed github release slug instead of panicking#461
arpitjain099 wants to merge 1 commit into
carvel-dev:developfrom
arpitjain099:fix/github-release-slug-format

Conversation

@arpitjain099

Copy link
Copy Markdown

fetchTagSelection splits opts.Slug on / and indexes element 1 without checking the result:

ownerName := strings.Split(d.opts.Slug, "/")[0]
repoName := strings.Split(d.opts.Slug, "/")[1]

Nothing between vendir.yml and that line checks the slug's shape. DirectoryContentsGithubRelease.Slug is a plain string with an // e.g. organization/repository comment, and I could not find a format check anywhere in the tree. So a config that omits the organization crashes the sync:

githubRelease:
  slug: myrepo
  tagSelection:
    semver: {constraints: ">=1.0.0"}
panic: runtime error: index out of range [1] with length 1

The split runs before the first API call, so it happens with no network access and no token configured.

strings.Cut now covers it, and empty owner or repo halves (/repo, owner/) are rejected too. The message names the expected format so the fix is obvious from the output.

The test drives fetchTagSelection directly for all four malformed shapes, which needs no client. It panics without the change.

fetchTagSelection splits opts.Slug on / and indexes element 1 without
checking. Nothing validates the slug's shape between vendir.yml and that
line, so a config with a missing organization, e.g.

  githubRelease:
    slug: myrepo
    tagSelection: {...}

crashes the sync:

  panic: runtime error: index out of range [1] with length 1

The split happens before the API call, so it fails with no network and
without a token. Return the format error instead.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants