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

Set permissions for /gcl-builds to a+rwx #1037

Closed
wants to merge 1 commit into from
Closed

Conversation

hatvik
Copy link

@hatvik hatvik commented Nov 15, 2023

Fixes #1036

@firecow
Copy link
Owner

firecow commented Nov 21, 2023

This should not be merged. It's not how Gitlab handles file permissions.

With DISABLE_UMASK

---
test-job:
  tags: [shared-docker-executor]
  image: alpine
  variables:
    FF_DISABLE_UMASK_FOR_DOCKER_EXECUTOR: 'true'
  script:
    - ls -all
    - ./script.sh

image

Without DISABLE_UMASK

---
test-job:
  tags: [shared-docker-executor]
  image: alpine
  variables:
    FF_DISABLE_UMASK_FOR_DOCKER_EXECUTOR: 'false'
  script:
    - ls -all
    - ./script.sh

image

@hatvik
Copy link
Author

hatvik commented Nov 23, 2023

Let me try to explain the issue we have. First, the premises are that we want to limit permissions to our source code. So adding more permissions to other is not an option. And the pipeline works in gitlab, without using root, but not in gcl.

Take this example:

ls -ld pr1037*
drwxr-x--- 2 eivin sudo 4096 nov.  23 08:07 pr1037
-rwxr-x--- 1 eivin sudo   35 nov.  23 08:07 pr1037.sh

ls -la pr1037/file_in_dir
-rw-r----- 1 eivin sudo 0 nov.  23 08:07 pr1037/file_in_dir
before_script:
  - ls -ld pr1037*
  - ./pr1037.sh || true
  - ls -la pr1037/file_in_dir
  - exit 0

gcl print_env

print_env                       $ ls -ld pr1037*
print_env                       > drwxrwxrw- 2 root root 4096 Nov 23 08:07 pr1037
print_env                       > -rwxrwxrw- 1 root root   35 Nov 23 08:07 pr1037.sh
print_env                       $ ./pr1037.sh || true
print_env                       > Running script
print_env                       $ ls -la pr1037/file_in_dir
print_env                       > -rw-rw-rw- 1 root root 0 Nov 23 08:07 pr1037/file_in_dir

gcl --umask false print_env

print_env                       $ ls -ld pr1037*
print_env                       > drwxrwxrw- 2 root root 4096 Nov 23 08:07 pr1037
print_env                       > -rwxrwxrw- 1 root root   35 Nov 23 08:07 pr1037.sh
print_env                       $ ./pr1037.sh || true
print_env                       $ ls -la pr1037/file_in_dir
print_env                       > /gcl-cmd: line 6: ./pr1037.sh: Permission denied
print_env                       > ls: cannot access 'pr1037/file_in_dir': Permission denied

Script is not run and we cannot access file inside dir pr1037

But you are saying we need to add +x to all scripts and directories?

You are correct, this is not how gitlab handles permissions, but this is how gcl handles them. And you handle them by doing a+rw on the /gcl-builds directory.
"chown 0:0 -R /gcl-builds/ && chmod a+rw -R /gcl-builds/ && chmod a+rw -R /tmp/",

Perhaps we should not do chown 0:0 here, when using --umask false?

@firecow
Copy link
Owner

firecow commented Nov 23, 2023

#1039

This PR will make GCL do exactly what Gitlab Docker Executor does.

I will let you know, when a new version is out, so you can try that out.

I'm closing this PR

@firecow firecow closed this Nov 23, 2023
@hatvik
Copy link
Author

hatvik commented Nov 23, 2023

#1039

This PR will make GCL do exactly what Gitlab Docker Executor does.

I will let you know, when a new version is out, so you can try that out.

I'm closing this PR

Yes you may close it, but i dont see how this will fix our issue.

You are still changing owner:group to 0:0, we do not want/cannot to run as root. We run in gitlab CI as the user specified in our docker image, which is a normal user.

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.

Using --umask false causes permission errors on /gcl-builds
2 participants