Better error messages for unauthorized PATs to private packages

[anderssonw]

Description

disclaimer: im not sure if this is possible / makes sense considering how PATs work

Trying to sign an image using a PAT gave me this error:

Error: signing [ghcr.io/ORG/IMAGE@sha256:DIGEST]: accessing image: entity not found in registry
main.go:74: error during command execution: signing [ghcr.io/ORG/IMAGE@sha256:DIGEST]: accessing image: entity not found in registry

I was sort of stumped as to why it happened, but it simply turned out I had not allowed the PAT access to my organization via the SAML login.

On a public package, this error was handled quite well, telling me that the PAT was not "logged in" using SAML. However, only a 404-esque response was given for private packages.

[fool1280]

Hi, can I take this issue?

[vishal-chdhry]

Hi @fool1280, are you working on it? If you are busy then I can pick this up!

[vishal-chdhry]

Assuming that @fool1280 is inactive, I am starting to work on it.

@haydentherapper @znewman01 Is there a specific reason why we are passing a hardcoded string here? The error string should be more descriptive

cosign/pkg/oci/remote/remote.go

Lines 52 to 56 in 8df14d8

	if errors.As(err, &te) && te.StatusCode == http.StatusNotFound {
	return nil, ErrEntityNotFound
	} else if err != nil {
	return nil, err
	}

ErrEntityNotFound = errors.New("entity not found in registry")

turned out I had not allowed the PAT access to my organization via the SAML login.

I think this should return a 403. But the if statement finds a 404 in the tree and returns it.

The error string in the issue is: accessing image: entity not found in registry.

Which means ErrEntityNotFound is returned in line 185

cosign/cmd/cosign/cli/sign/sign.go

Lines 182 to 185 in 8df14d8

	if err == ociremote.ErrEntityNotFound {
	se = ociremote.SignedUnknown(digest)
	} else if err != nil {
	return fmt.Errorf("accessing image: %w", err)

(we are only returning accessing image: in line 185 so that must be how)

But we are also checking in the if statement if the error is not of type ErrEntityNotFound in the above if statement. Is the logic in 182 working?

@anderssonw Are you using the latest version?

[anderssonw]

I haven't used the cosign CLI since posting this issue, so not sure what the message is now, sorry!

[znewman01]

On a public package, this error was handled quite well, telling me that the PAT was not "logged in" using SAML. However, only a 404-esque response was given for private packages.

My hunch is that this is by design. It's pretty common for APIs to return 404s instead of 403s for private entities, in order to prevent unauthorized users for learning whether an entity exists. If that's the case, I don't know how much we can do about it client-side.

Is there a specific reason why we are passing a hardcoded string here? The error string should be more descriptive

We'd be okay with a more descriptive error if you have a specific proposal.

[vishal-chdhry]

@znewman01 Sorry for the delay, I was busy with academic stuff.

That exact error cannot happen anymore as that behavior was changed in v2.1.0. But I still believe we should change the error to be more descriptive as ociremote.SignedEntity is used elsewhere as well and with the current design, the error value from remote get is lost.

Sending a PR.