| Message ID | 20250307120055.1816436-1-l.goehrs@pengutronix.de |
|---|---|
| Headers | show |
| Series | fetch2/github_release_artifact: fetcher for (private) release artifacts | expand |
On Fri, 2025-03-07 at 13:00 +0100, Leonard Göhrs via lists.yoctoproject.org wrote: > This fetcher enables downloading artifacts attached to GitHub releases > in private repositories. > > Authentication is provided using tokens via the `BB_FETCH_GHRA_TOKEN` > variable. > > Changes from v1 to v2: > > - Provide examples on how to provide the `GH_TOKEN` more securely than > just hardcoding it in the recipe in the commit message. > E.g. via the `local.conf` or environment variables. > - Indent the code example in the documentation `.rst` using three > spaces instead of two. > - Document the `token=` URI parameter along with the `GH_TOKEN` > variable. > > Changes from v2 to v3: > > - Rename `FetchData.headers` to `FetchData.http_headers` and do not set > a default. Use `getattr` with a default instead. > - Rename the token variable from `GH_TOKEN` (the de-facto default in > GitHub actions) to `BB_FETCH_GHRA_TOKEN` (makes it more obvious where > the variable is used). > - Remove support for supplying the GitHub token as SRC_URI parameter > to make it less likely that they are accidentally leaked, when e.g. > sharing the recipe. > - Do not use `dict_a | dict_b` to merge the header dicts, > as that feature is not available in Python 3.8 used on Ubuntu 20.04. > Use `dict(**dict_a, **dict_b)` instead. > - Explicitly state in the documentation that access tokens should have > minimal scope. > > Alexander Kanavin and Peter Marko have voiced some security concerns about > the token handling in bitbake variables that have not been adressed in > this v3. > > The security of the current solution should be on par or better than > what we have for `http` URLs that include basic auth usernames and > passwords. I consider this acceptable. > > A generic way to store these kinds of fetcher secrets outside of the > normal bitbake variables would be desirable though. I've been giving this patchset a lot of thought. I'm a bit concerned about creating a new "fetcher" namespace for something that is fairly niche, particularly when as Peter points out, the same thing can be achieved with a bbclass. If there were no other way to do this, I think I'd be leaning to taking the patches but since there is another way which isn't too difficult, I think that we should promote that approach instead. One of the reasons for this is that by merging the new fetcher, it would complicate potential future changes to the wget fetcher and I'd prefer not to further limit our options there if we can help it. Unless I see other people saying they need this new fetcher, I'm therefore likely going to decline it in favour of the bbclass approach. Cheers, Richard
On Mon, Mar 10, 2025 at 9:53 AM Richard Purdie via lists.yoctoproject.org <richard.purdie=linuxfoundation.org@lists.yoctoproject.org> wrote: > > On Fri, 2025-03-07 at 13:00 +0100, Leonard Göhrs via lists.yoctoproject.org wrote: > > This fetcher enables downloading artifacts attached to GitHub releases > > in private repositories. > > > > Authentication is provided using tokens via the `BB_FETCH_GHRA_TOKEN` > > variable. > > > > Changes from v1 to v2: > > > > - Provide examples on how to provide the `GH_TOKEN` more securely than > > just hardcoding it in the recipe in the commit message. > > E.g. via the `local.conf` or environment variables. > > - Indent the code example in the documentation `.rst` using three > > spaces instead of two. > > - Document the `token=` URI parameter along with the `GH_TOKEN` > > variable. > > > > Changes from v2 to v3: > > > > - Rename `FetchData.headers` to `FetchData.http_headers` and do not set > > a default. Use `getattr` with a default instead. > > - Rename the token variable from `GH_TOKEN` (the de-facto default in > > GitHub actions) to `BB_FETCH_GHRA_TOKEN` (makes it more obvious where > > the variable is used). > > - Remove support for supplying the GitHub token as SRC_URI parameter > > to make it less likely that they are accidentally leaked, when e.g. > > sharing the recipe. > > - Do not use `dict_a | dict_b` to merge the header dicts, > > as that feature is not available in Python 3.8 used on Ubuntu 20.04. > > Use `dict(**dict_a, **dict_b)` instead. > > - Explicitly state in the documentation that access tokens should have > > minimal scope. > > > > Alexander Kanavin and Peter Marko have voiced some security concerns about > > the token handling in bitbake variables that have not been adressed in > > this v3. > > > > The security of the current solution should be on par or better than > > what we have for `http` URLs that include basic auth usernames and > > passwords. I consider this acceptable. > > > > A generic way to store these kinds of fetcher secrets outside of the > > normal bitbake variables would be desirable though. > > I've been giving this patchset a lot of thought. I'm a bit concerned > about creating a new "fetcher" namespace for something that is fairly > niche, particularly when as Peter points out, the same thing can be > achieved with a bbclass. > > If there were no other way to do this, I think I'd be leaning to taking > the patches but since there is another way which isn't too difficult, I > think that we should promote that approach instead. I think, the bbclass abstraction approach would also make it expandable to other git infra's which might be used in private settings e.g. github etc. so if it was possible, I would incline towards the bbclass approach. > > One of the reasons for this is that by merging the new fetcher, it > would complicate potential future changes to the wget fetcher and I'd > prefer not to further limit our options there if we can help it. > > Unless I see other people saying they need this new fetcher, I'm > therefore likely going to decline it in favour of the bbclass approach. > > Cheers, > > Richard > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#6524): https://lists.yoctoproject.org/g/docs/message/6524 > Mute This Topic: https://lists.yoctoproject.org/mt/111566036/1997914 > Group Owner: docs+owner@lists.yoctoproject.org > Unsubscribe: https://lists.yoctoproject.org/g/docs/unsub [raj.khem@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- >
This fetcher enables downloading artifacts attached to GitHub releases in private repositories. Authentication is provided using tokens via the `BB_FETCH_GHRA_TOKEN` variable. Changes from v1 to v2: - Provide examples on how to provide the `GH_TOKEN` more securely than just hardcoding it in the recipe in the commit message. E.g. via the `local.conf` or environment variables. - Indent the code example in the documentation `.rst` using three spaces instead of two. - Document the `token=` URI parameter along with the `GH_TOKEN` variable. Changes from v2 to v3: - Rename `FetchData.headers` to `FetchData.http_headers` and do not set a default. Use `getattr` with a default instead. - Rename the token variable from `GH_TOKEN` (the de-facto default in GitHub actions) to `BB_FETCH_GHRA_TOKEN` (makes it more obvious where the variable is used). - Remove support for supplying the GitHub token as SRC_URI parameter to make it less likely that they are accidentally leaked, when e.g. sharing the recipe. - Do not use `dict_a | dict_b` to merge the header dicts, as that feature is not available in Python 3.8 used on Ubuntu 20.04. Use `dict(**dict_a, **dict_b)` instead. - Explicitly state in the documentation that access tokens should have minimal scope. Alexander Kanavin and Peter Marko have voiced some security concerns about the token handling in bitbake variables that have not been adressed in this v3. The security of the current solution should be on par or better than what we have for `http` URLs that include basic auth usernames and passwords. I consider this acceptable. A generic way to store these kinds of fetcher secrets outside of the normal bitbake variables would be desirable though.