Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
iamstarkov opened this issue Aug 2, 2022 · 1 comment
Assignees
Labels
bug Something isn't working
Milestone

Comments

@iamstarkov
Copy link

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 iamstarkov/codeowners-validator-issue-demo#1

Troubleshooting

I don't know what to put here

@iamstarkov iamstarkov added the bug Something isn't working label Aug 2, 2022
@mszostok
Copy link
Owner

mszostok commented Aug 5, 2022

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 mszostok added this to the Next milestone Aug 5, 2022
@mszostok mszostok modified the milestones: Next, v0.8.0 Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants