Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid listing org packages where job token doesn't have access #27

Closed
wants to merge 4 commits into from

Conversation

mering
Copy link
Contributor

@mering mering commented Aug 22, 2023

Follow-up on #23 but now including a fix to taking the special case into account where package_name doesn't exist.

Unfortunately, I can't run the tests as I don't have access to create the packages for testing (403 permission denied).


This allows using the job token directly.

Example workflow:

permissions:
  packages: write

jobs:
  cleanup-images:
    runs-on: ubuntu-22.04
    steps:
      - name: Cleanup untagged images
        uses: mering/delete-untagged-ghcr-action@main
        with:
          token: ${{ github.token }}
          repository_owner: ${{ github.repository_owner }}
          repository: ${{ github.repository }}
          package_name: IMAGE_NAME
          untagged_only: true
          owner_type: org

@mering
Copy link
Contributor Author

mering commented Oct 30, 2023

@Chizkiyahu friendly ping

@Chizkiyahu Chizkiyahu self-assigned this Oct 30, 2023
@Chizkiyahu
Copy link
Owner

@mering
Thank you for the ping and sorry
fails logs

Traceback (most recent call last):
  File "/home/runner/work/delete-untagged-ghcr-action/delete-untagged-ghcr-action/.//clean_ghcr.py", line 237, in <module>
    delete_pkgs(
  File "/home/runner/work/delete-untagged-ghcr-action/delete-untagged-ghcr-action/.//clean_ghcr.py", line 149, in delete_pkgs
    packages = get_list_packages(
  File "/home/runner/work/delete-untagged-ghcr-action/delete-untagged-ghcr-action/.//clean_ghcr.py", line 60, in get_list_packages
    response = requests.get(url, headers=get_base_headers(), params=params)
NameError: name 'params' is not defined```

if the tests are falling try to run them on your fork 
with `PAT_TOKEN`  secret

@mering mering force-pushed the allow-gh-actions-token branch 3 times, most recently from ad8b5cc to ef95667 Compare October 31, 2023 11:18
@mering
Copy link
Contributor Author

mering commented Oct 31, 2023

Latest version has been tested in my fork and all tests pass. Prefer GITHUB_TOKEN where possible.

@mering
Copy link
Contributor Author

mering commented Oct 31, 2023

Oh, I see that the maximum permissions for pull GITHUB_TOKEN for pull requests from public forked repositories are read according to the docs. I will revert the test workflow to using the PAT secret.

@mering
Copy link
Contributor Author

mering commented Oct 31, 2023

This should be working now also for the PR workflow.

@Chizkiyahu Chizkiyahu mentioned this pull request Oct 31, 2023
@mering
Copy link
Contributor Author

mering commented Oct 31, 2023

Running workflows from forks seems difficult. I updated the user name to match the one from PAT.

@Chizkiyahu
Copy link
Owner

@mering
It look like something is broken in "delete all package in repo"
my test are not really a good test
because they not check if pkg is created and deleted
you needs to check that manual
I am debugging that now

@mering
Copy link
Contributor Author

mering commented Oct 31, 2023

I checked and access to secrets isn't possible for pull requests from public forks....

I guess the problem is that a package does not have the repository property when it is not uploaded with GITHUB_TOKEN but this token never has write permissions when running from a public fork. I reverted the last two commits and confirmed in my repo that this fixes the problem (you need to manually clean all packages before starting the run as the job token doesn't have access to remove an image created with the PAT).

@Chizkiyahu
Copy link
Owner

@mering
I merged #30
I change some stuff because of bugs (yes my test can improve). but the code broke them

@Chizkiyahu
Copy link
Owner

@mering
please open issues if my code does not solve your problem
or you found any bug

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants