Skip to content

fix sub repo filtering for sub-dir of repo

Warren Gifford requested to merge mlw-fix-ReadDir-sub-repo-filtering into main

Created by: mollylogue

See https://docs.google.com/document/d/1nw-P1ZOgR5qYOe725vm9xNnRRWYRucdeSXeblsG8seQ/edit for more context. Essentially, the ReadDir function makes a git ls-tree call, which returns a list of files/directories, where the directories do not have trailing slashes. However, when permissions to view a directory in perforce is revoked, the trailing slash is present. The rule from perforce looks something like read group <group name> * -//app/… which translates to a path_excludes in psql that looks like {app/**}, and the path that's passed in in this case is "app".

This leads to these two paths not matching and thus filtering working incorrectly. I've remedied that by adding the trailing slash to the directories before checking permissions. We could loop through and add this trailing slash this right after the git ls-tree command rather than inside the authz package if people prefer (I just wanted to avoid looping through all the paths more than necessary).

Questions:

  1. Could we potentially see something like read group <group name> * -//app… ? What would that mean? It looks like in practice we aren't seeing this pattern.
  2. What's the best unit test strategy for this? I was thinking of creating a test for ReadDir and not using the normal mock sub repo perms checker but rather mocking at the sub-repo permissions getter layer where we fetch from the db, just to effectively test the matching logic.

Test plan

Tested locally already, validated that the directory the user shouldn't have access to is now no longer visible. Will also need to validate that this works for the customer test case.

Merge request reports

Loading