search

octo-sts / app

A GitHub App that acts like a Security Token Service (STS) for the Github API
Apache License 2.0
126 stars 15 forks source link

Why doesn't Octo STS output the error detail? #246

Open suzuki-shunsuke opened 5 months ago

suzuki-shunsuke commented 5 months ago

Hi, thank you for your great project! I have a question about this app.

Some error messages of this App aren't helpful. For example, when this App can't parse trust policy, the App outputs Error: unable to parse trust policy found for "***" but we can't understand why.

e.g.

Run octo-sts/action@6177b4481c00308b3839969c3eca88c96a91775f
Error: unable to parse trust policy found for "foo"
image

I checked the source code then I found this App hides the error detail intentionally.

https://github.com/octo-sts/app/blob/1fc549c0973c27a9aba0257626d1903486dee2d9/pkg/octosts/octosts.go#L311-L315

Don't leak the error to the client.

But I'm not sure why. Why does this App hide an error detail? Do we need to hide Trust Policy from clients?

Error detail would be helpful for troubleshooting.

mattmoor commented 5 months ago

The policy may live in a private repo, and since we haven't authorized the caller we don't want to leak any data back to the caller.

@wlynch had some work to add a validating webhook for policies, which I think may have gotten closed when I moved this into its own org, but we should potentially revisit as a way to check things before they are merged into the repository vs. at STS time.

suzuki-shunsuke commented 5 months ago

Thank you for your quick answer!

The policy may live in a private repo, and since we haven't authorized the caller we don't want to leak any data back to the caller.

I see. It makes sense.

wlynch had some work to add a validating webhook for policies, which I think may have gotten closed when I moved this into its own org

Are you talking about this pull request?

wlynch closed this by deleting the head repository on Mar 13

mattmoor commented 5 months ago

Yeah that’s the one.

wlynch commented 5 months ago

Revived at https://github.com/octo-sts/app/pull/247!