mszostok / codeowners-validator

The GitHub CODEOWNERS file validator
Apache License 2.0
212 stars 48 forks source link

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

Open iamstarkov opened 1 year ago

iamstarkov commented 1 year ago

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/*  docs@example.com
/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

mszostok commented 1 year ago

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 👍 .