codeowners-validator icon indicating copy to clipboard operation
codeowners-validator copied to clipboard

/folder/* wildcard pattern is assumed to be nested by the action "Not Owned" check, which it isnt

Open iamstarkov opened this issue 3 years ago • 1 comments

Description

Let's say you have the repository with the given file structure:

~/projects/oss/codeowners-validator-issue-demo (main) $ tree .
.
├── CODEOWNERS
└── packages
    └── demo
        ├── index.js
        ├── package.json
        └── src
            ├── not-owned
            │   └── index.js
            └── owned
                └── index.js

and given CODEOWNERS file:

~/projects/oss/codeowners-validator-issue-demo (main) $ cat CODEOWNERS 
# Docs https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners

# The `docs/*` pattern will match files like
# `docs/getting-started.md` but not further nested files like
# `docs/build-app/troubleshooting.md`.
# docs/*  [email protected]
/packages/demo/* @iamstarkov


# In this example, @doctocat owns any files in the build/logs
# directory at the root of the repository and any of its
# subdirectories.
# /build/logs/ @doctocat
/packages/demo/src/owned @iamstarkov


# Current configuration leaves files in /packages/demo/src/not-owned folder not owned, but validator fails to throw an error
# /packages/demo/src/not-owned @iamstarkov

The problem with this action is that despite /packages/demo/src/not-owned is not owned, because /packages/demo/*doesn't provide ownership for nested files, codeowners-validator action doesn't fail the "not owned" check.

And wildcard pattern misinterpretation most likely is the root cause, because if you remove it, action does recognise the problem and fails the check.

Expected result

given original file structure and codeowners configuration from the description or from the main branch of the demo repo, then not owned check should fail.

Maybe other semantic rules are worth to be checked too.

Actual result

clearly not owned files don't make the check fail.

Steps to reproduce

see demo repo for reproduction https://github.com/iamstarkov/codeowners-validator-issue-demo/ and 1st PR too https://github.com/iamstarkov/codeowners-validator-issue-demo/pull/1/

Troubleshooting

I don't know what to put here

iamstarkov avatar Aug 02 '22 12:08 iamstarkov

Hi @iamstarkov

Thanks for reporting that. Some time ago (> 1 year 😅 ) try to fix that: https://github.com/mszostok/codeowners-validator/pull/69/files

However, this path traversal is quite tricky and I wanted to do more testing to do not introduce a new bug instead. I will add a prio on it and try to get it merge soon 👍 .

mszostok avatar Aug 05 '22 13:08 mszostok