[BUG] This action inside a workflow executed inside a container doesn't work

[NikitaCOEUR]

This line:

github-action-terraform-init/main.sh

Line 20 in a0f2e06

if [ "$exist_lock_file" = "false" ] || ! git diff --quiet .terraform.lock.hcl; then

causes this action to fail when executed through a GitHub Action workflow running inside a container due to a couple of problems:

https://github.com/actions/checkout/issues/956
https://github.blog/2022-04-12-git-security-vulnerability-announced/#cve-2022-24765

The owner of the folder containing the repository checked out inside a container is the user who runs the container. However, the user who clones the content of the repo with the GitHub Action checkout is "root" as defined here: https://docs.github.com/en/actions/creating-actions/dockerfile-support-for-github-actions#user.

The workaround seems very simple:

if [ "$exist_lock_file" = "false" ] || ! git diff --quiet .terraform.lock.hcl; then 

Should be changed to :

if [ "$exist_lock_file" = "false" ] || ! git diff --quiet -C "$ROOT_DIR/$WORKING_DIR" .terraform.lock.hcl; then

[suzuki-shunsuke]

• Experiencing permission denied after v3.1.0 on jobContainers when container user is not 'root' actions/checkout#956
• https://github.blog/2022-04-12-git-security-vulnerability-announced/#cve-2022-24765

--

https://docs.github.com/en/actions/creating-actions/dockerfile-support-for-github-actions#user

This document is about Dockerfile support, but this action doesn't use Dockerfile.

[NikitaCOEUR]

Not this action ! but I use this action inside a container...

[NikitaCOEUR]

Not this action ! but I use this action inside a container...

With the action checkout as mentioned in my issue

[suzuki-shunsuke]

I see.
I could reproduce the issue of actions/checkout.

• test container test-github-action-2#17

[suzuki-shunsuke]

Do you use a Docker image whose USER is a non root user?
How do you run actions/checkout?

[NikitaCOEUR]

Do you use a Docker image whose USER is a non root user?
How do you run actions/checkout?

I use a docker image without USER in Dockerfile, so with root user.

And I run action/checkout inside container.

[suzuki-shunsuke]

I see. Thank you. I'm trying to understand your issue correctly.

I ran the following workflow, then git diff command failed due to warning: Not a git repository.

jobs:
  container-test-job:
    runs-on: ubuntu-latest
    container:
      image: "debian:bookworm-20240701"
    steps:
      - run: whoami
      - run: apt-get update
      - run: apt-get install -y git
      - uses: actions/checkout@v4
      - run: ls -lh
      - run: git diff --quiet README.md

https://github.com/suzuki-shunsuke/test-github-action-2/actions/runs/9987965762/job/27603427863

Run git diff --quiet README.md
warning: Not a git repository. Use --no-index to compare two paths outside a working tree
usage: git diff --no-index [<options>] <path> <path>

Diff output format options
    -p, --patch           generate patch
    -s, --no-patch        suppress diff output
    -u                    generate patch
    -U, --unified[=<n>]   generate diffs with <n> lines context
    -W, --function-context
                          generate diffs with <n> lines context
    --raw                 generate the diff in raw format
    --patch-with-raw      synonym for '-p --raw'
    --patch-with-stat     synonym for '-p --stat'
    --numstat             machine friendly --stat
    --shortstat           output only the last line of --stat
    -X, --dirstat[=<param1,param2>...]
                          output the distribution of relative amount of changes for each sub-directory
    --cumulative          synonym for --dirstat=cumulative
    --dirstat-by-file[=<param1,param2>...]
                          synonym for --dirstat=files,param1,param2...
    --check               warn if changes introduce conflict markers or whitespace errors
    --summary             condensed summary such as creations, renames and mode changes
    --name-only           show only names of changed files
    --name-status         show only names and status of changed files
    --stat[=<width>[,<name-width>[,<count>]]]
                          generate diffstat
    --stat-width <width>  generate diffstat with a given width
    --stat-name-width <width>
                          generate diffstat with a given name width
    --stat-graph-width <width>
                          generate diffstat with a given graph width
    --stat-count <count>  generate diffstat with limited lines
    --compact-summary     generate compact summary in diffstat
    --binary              output a binary diff that can be applied
    --full-index          show full pre- and post-image object names on the "index" lines
    --color[=<when>]      show colored diff
    --ws-error-highlight <kind>
                          highlight whitespace errors in the 'context', 'old' or 'new' lines in the diff
    -z                    do not munge pathnames and use NULs as output field terminators in --raw or --numstat
    --abbrev[=<n>]        use <n> digits to display object names
    --src-prefix <prefix>
                          show the given source prefix instead of "a/"
    --dst-prefix <prefix>
                          show the given destination prefix instead of "b/"
    --line-prefix <prefix>
                          prepend an additional prefix to every line of output
    --no-prefix           do not show any source or destination prefix
    --inter-hunk-context <n>
                          show context between diff hunks up to the specified number of lines
    --output-indicator-new <char>
                          specify the character to indicate a new line instead of '+'
    --output-indicator-old <char>
                          specify the character to indicate an old line instead of '-'
    --output-indicator-context <char>
                          specify the character to indicate a context instead of ' '

Diff rename options
    -B, --break-rewrites[=<n>[/<m>]]
                          break complete rewrite changes into pairs of delete and create
    -M, --find-renames[=<n>]
                          detect renames
    -D, --irreversible-delete
                          omit the preimage for deletes
    -C, --find-copies[=<n>]
                          detect copies
    --find-copies-harder  use unmodified files as source to find copies
    --no-renames          disable rename detection
    --rename-empty        use empty blobs as rename source
    --follow              continue listing the history of a file beyond renames
    -l <n>                prevent rename/copy detection if the number of rename/copy targets exceeds given limit

Diff algorithm options
    --minimal             produce the smallest possible diff
    -w, --ignore-all-space
                          ignore whitespace when comparing lines
    -b, --ignore-space-change
                          ignore changes in amount of whitespace
    --ignore-space-at-eol
                          ignore changes in whitespace at EOL
    --ignore-cr-at-eol    ignore carrier-return at the end of line
    --ignore-blank-lines  ignore changes whose lines are all blank
    -I, --ignore-matching-lines <regex>
                          ignore changes whose all lines match <regex>
    --indent-heuristic    heuristic to shift diff hunk boundaries for easy reading
    --patience            generate diff using the "patience diff" algorithm
    --histogram           generate diff using the "histogram diff" algorithm
    --diff-algorithm <algorithm>
                          choose a diff algorithm
    --anchored <text>     generate diff using the "anchored diff" algorithm
    --word-diff[=<mode>]  show word diff, using <mode> to delimit changed words
    --word-diff-regex <regex>
                          use <regex> to decide what a word is
    --color-words[=<regex>]
                          equivalent to --word-diff=color --word-diff-regex=<regex>
    --color-moved[=<mode>]
                          moved lines of code are colored differently
    --color-moved-ws <mode>
                          how white spaces are ignored in --color-moved

Other diff options
    --relative[=<prefix>]
                          when run from subdir, exclude changes outside and show relative paths
    -a, --text            treat all files as text
    -R                    swap two inputs, reverse the diff
    --exit-code           exit with 1 if there were differences, 0 otherwise
    --quiet               disable all output of the program
    --ext-diff            allow an external diff helper to be executed
    --textconv            run external text conversion filters when comparing binary files
    --ignore-submodules[=<when>]
                          ignore changes to submodules in the diff generation
    --submodule[=<format>]
                          specify how differences in submodules are shown
    --ita-invisible-in-index
                          hide 'git add -N' entries from the index
    --ita-visible-in-index
                          treat 'git add -N' entries as real in the index
    -S <string>           look for differences that change the number of occurrences of the specified string
    -G <regex>            look for differences that change the number of occurrences of the specified regex
    --pickaxe-all         show all changes in the changeset with -S or -G
    --pickaxe-regex       treat <string> in -S as extended POSIX regular expression
    -O <file>             control the order in which files appear in the output
    --rotate-to <path>    show the change in the specified path first
    --skip-to <path>      skip the output to the specified path
    --find-object <object-id>
                          look for differences that change the number of occurrences of the specified object
    --diff-filter [(A|C|D|M|R|T|U|X|B)...[*]]
                          select files by diff type
    --output <file>       output to a specific file

Error: Process completed with exit code 12[9](https://github.com/suzuki-shunsuke/test-github-action-2/actions/runs/9987965762/job/27603427863?pr=18#step:8:10).

The workflow succeeded if I didn't use a container.

jobs:
  container-test-job:
    runs-on: ubuntu-latest
    # container:
    #   image: "debian:bookworm-20240701"
    steps:
      - run: whoami
      # - run: apt-get update
      # - run: apt-get install -y git
      - uses: actions/checkout@v4
      - run: ls -lh
      - run: git diff --quiet README.md

[suzuki-shunsuke]

git diff succeeded if -C is used. Interesting.

https://github.com/suzuki-shunsuke/test-github-action-2/actions/runs/9988100119/job/27603835920

      - run: git diff -C "$ROOT_DIR" --quiet README.md
        env:
          ROOT_DIR: ${{ github.workspace }}

https://git-scm.com/docs/git

-C <path>
Run as if git was started in <path> instead of the current working directory. When multiple -C options are given, each subsequent non-absolute -C <path> is interpreted relative to the preceding -C <path>. If <path> is present but empty, e.g. -C "", then the current working directory is left unchanged.

This option affects options that expect path name like --git-dir and --work-tree in that their interpretations of the path names would be made relative to the working directory caused by the -C option. For example the following invocations are equivalent:

I'm not sure why -C option resolves the error.

I checked $PWD just in case, but it's same with ${{ github.workspace }}.

https://github.com/suzuki-shunsuke/test-github-action-2/actions/runs/9988239602/job/27604263211?pr=18

[suzuki-shunsuke]

• fatal: not a git repository when using checkout@v2 actions/checkout#363
• fatal: unsafe repository (REPO is owned by someone else) in other workflow steps after running checkout actions/checkout#766

git config --global --add safe.directory "$GITHUB_WORKSPACE" resolved the error.

      - run: git config --global --add safe.directory "$GITHUB_WORKSPACE"
      - run: git diff --quiet README.md

https://github.com/suzuki-shunsuke/test-github-action-2/actions/runs/9988435719/job/27604893593

[suzuki-shunsuke]

So I think this issue has nothing to do with this action.
I think you should run git config --global --add safe.directory "$GITHUB_WORKSPACE" after actions/checkout if you use a container.

[NikitaCOEUR]

Wow, nice debugging and good job thinking to try that workaround. Indeed, it should work that way!

However, I still think it's a good idea to adapt this action to make it work in both cases, even if the command: git config --global --add safe.directory "$GITHUB_WORKSPACE" hasn't been run beforehand, right?

[NikitaCOEUR]

I will use this workaround. You are the owner of this action, so you have the final say.

[suzuki-shunsuke]

Hmm, action/checkout has the option set-safe-directory and it is enabled by default.

https://github.com/actions/checkout

    # Add repository path as safe.directory for Git global config by running `git
    # config --global --add safe.directory <path>`
    # Default: true
    set-safe-directory: ''

I checked the action log, and it looks /usr/bin/git config --global --add safe.directory /__w/test-github-action-2/test-github-action-2 was run, but it didn't work. 🤔

https://github.com/suzuki-shunsuke/test-github-action-2/actions/runs/9988435719/job/27604893593#step:6:25

Adding repository directory to the temporary git global config as a safe directory
/usr/bin/git config --global --add safe.directory /__w/test-github-action-2/test-github-action-2

temporary git global config

Is this change temporary?

• fatal: unsafe repository (REPO is owned by someone else) in other workflow steps after running checkout actions/checkout#766

Why don't we persist the configuration we use in actions/checkout
We could try to persist this temporary global configuration we set in checkout for the duration of your job, but there are few problems with that:

If you run checkout on the root machine, and you have a container action with git commands, you are still going to fail unless you set the config in that container, which checkout can't do for another step
Overwriting the git global config and not persisting any changes back to the original global config may break some user expectations on self hosted runners.
It only really addresses this issue for checkout users, but this is more of an actions ecosystem problem

[suzuki-shunsuke]

I will use this workaround. You are the owner of this action, so you have the final say.

Thank you for your understanding. I close this issue.