Our current logic attempts to detect conflicting rules and remove them. There are potential situations
where this may remove rules too broadly and cause us to leak data.
Instead, we should simplify our current approach even if it's slower initially in order to guarantee correctness.
We can improve the performance later by optimising the code and adjusting the algorithm.
Instead of keeping a separate lists of include and exclude rules we keep one list of rules in the same order they appear in the protection table, just converted into a glob pattern. We can continue to filter out rules that don't apply to the current depot.
Then, for each path we just move down the list one rule at a time granting or revoking read access until we get to the end. Once we're at the end we'll know whether the path is allowed or not. We'd always start with a no access to be safe.
This will definitely be slower as we'll need to check all the rules (ie, O(n) where n is the number of rules) but it may be fast enough for our needs and is simpler and at least much easier to reason about.
For reference, our current logic to generate the list of rules is here
We may need to introduce a new database column in sub_repo_permissions to store all rules and use the new code path if it is populated or the old code path if not in order to allow a smooth migration for customers.
/cc @sourcegraph/iam
Todo
Our current logic attempts to detect conflicting rules and remove them. There are potential situations
where this may remove rules too broadly and cause us to leak data.
Instead, we should simplify our current approach even if it's slower initially in order to guarantee correctness.
We can improve the performance later by optimising the code and adjusting the algorithm.
Instead of keeping a separate lists of include and exclude rules we keep one list of rules in the same order they appear in the protection table, just converted into a glob pattern. We can continue to filter out rules that don't apply to the current depot.
Then, for each path we just move down the list one rule at a time granting or revoking read access until we get to the end. Once we're at the end we'll know whether the path is allowed or not. We'd always start with a no access to be safe.
This will definitely be slower as we'll need to check all the rules (ie, O(n) where n is the number of rules) but it may be fast enough for our needs and is simpler and at least much easier to reason about.
For reference, our current logic to generate the list of rules is here
We may need to introduce a new database column in
sub_repo_permissionsto store all rules and use the new code path if it is populated or the old code path if not in order to allow a smooth migration for customers./cc @sourcegraph/iam
Todo