gitlabform icon indicating copy to clipboard operation
gitlabform copied to clipboard

AttributeError when altering files on protected branch

Open margana opened this issue 8 months ago • 3 comments

Describe the bug

File "/gitlabform/gitlabform/processors/project/files_processor.py", line 211, in modify_file_dealing_with_branch_protection
    self.just_modify_file(
  File "/gitlabform/gitlabform/processors/project/files_processor.py", line 242, in just_modify_file
    commit_message=self.get_commit_message_for_file_change(
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/gitlabform/gitlabform/processors/project/files_processor.py", line 281, in get_commit_message_for_file_change
    commit_message = configuration.get(
                     ^^^^^^^^^^^^^^^^^^
  File "/gitlabform/gitlabform/processors/util/decorators.py", line 25, in get
    val = val.get(key, default)
          ^^^^^^^

The SafeDict decorator fails here because while resolving "files|<file>|commit_message" the filename isn't found, val gets set to the default commit string and then treated as a dict when it tries to get commit_message.

The reason it can't find <file> is due to a side effect in gitlab.v4.objects.ProjectFile where it mutates self.file_path upon save():

https://github.com/python-gitlab/python-gitlab/blob/v5.6.0/gitlab/v4/objects/files.py#L74

The mutated value is then re-used in modify_file_dealing_with_branch_protection when retrying the file operation after unlocking the branch:

https://github.com/gitlabform/gitlabform/blob/2d7589121be6b8677f2adef2e3822b877698276c/gitlabform/processors/project/files_processor.py#L210-L214

GitLabForm version

🏗 GitLabForm version: 4.2.0 = the latest stable 😊

GitLab version

[v17.10.4-ee]

margana avatar Apr 17 '25 22:04 margana

Hi @margana - Thanks for the report and doing all the investigation. It'd be good to provide some more context of the issue before going into the solution (e.g. sample gitlabform config, steps to reproduce the issue). Could you please provide those info?

Aside, I believe there are already test cases that tries to modify files in a protected branch. I'm curious why those tests are not failing. This is where what I mentioned above would be helpful.

https://github.com/gitlabform/gitlabform/blob/2d7589121be6b8677f2adef2e3822b877698276c/tests/acceptance/standard/test_files.py

Assuming it's a bug, would you be able to help contribute a fix? Might be a good idea to try identify the gaps in existing test cases and then add the test.

amimas avatar Apr 19 '25 15:04 amimas

The test cases don't hit this bug because the mutation is url encoding and the tested filenames don't need encoding. If you for example include a slash in the file_path it should fail.

Unfortunately, my experience with python is pretty minimal. I didn't want to just add an api call to request a new ProjectFile object so I tried to deepcopy it before the first call but that didn't work and I ended up just commenting out the entire call since I only need protected branches.

margana avatar Apr 19 '25 19:04 margana

Thanks. As requested, could you please share your gitlabform config?

amimas avatar Jul 26 '25 15:07 amimas