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

git_blacklist never runs? #1139

Open
cerw opened this issue Jun 10, 2024 · 5 comments
Open

git_blacklist never runs? #1139

cerw opened this issue Jun 10, 2024 · 5 comments
Labels

Comments

@cerw
Copy link

cerw commented Jun 10, 2024

Q A
Version GrumPHP 2.5.0
Bug? yes
New feature? no
Question? no
Documentation? no
Related tickets

We noticed that the git_blacklist is not executing it seem its because canRunInContext always return false?

My configuration

grumphp:
  stop_on_failure: true
  process_timeout: 210
  parallel:
    enabled: true
    max_workers: 32
  tasks:
    file_size:
      max_size: 3M
      ignore_patterns: []
    phpcs:
      standard: PSR2
      warning_severity: 0
      error_severity: 1
      ignore_patterns:
        - ./tests
        - ./resources/*
        - ./database
        - ./bootstrap
        - ./public
        - ./config/
    git_branch_name:
      # whitelist:
      #   - '/([0-9]+)-/'
      #   - '/bugfix-*/'
      blacklist:
        - "/(shit|fuck|crap)/"
      # - "develop"
      # - "master"
    git_commit_message:
      allow_empty_message: false
      enforce_capitalized_subject: false
      enforce_no_subject_trailing_period: true
      enforce_single_lined_subject: false
      max_body_width: 400
      max_subject_width: 200
      case_insensitive: true
      multiline: true
      additional_modifiers: ""
    jsonlint: null
    phplint: null
    securitychecker_enlightn:
      lockfile: ./composer.lock
    git_blacklist:
      keywords:
        - "die("
        - "dump("
        - "dd("
        - "var_dump("
        - "exit;"
      # whitelist_patterns:
      #   - add(
      regexp_type: G
      match_word: true
    composer:

Steps to reproduce:

Put dd(); in PHP code, do git add -y run following

nanospa git:(last_bits_on_spa) ✗ git diff --cached app

─────────────────────────────────────────────────────────────────────────────────┐
app/Listeners/LogUserHasLoggedOff.php:29: public function handle(Logout $logout) │
─────────────────────────────────────────────────────────────────────────────────┘
 29 ⋮ 29 │            // no user is logged in
 30 ⋮ 30 │            return;
 31 ⋮ 31 │        }
    ⋮ 32 │        dd();
 32 ⋮ 33 │        activity()
 33 ⋮ 34 │            ->useLog('auth')
 34 ⋮ 35 │            ->event('logout')
./vendor/bin/grumphp run 


# Your actions
# Please add the steps on how to reproduce the issue here.

# Run GrumPHP:
git add -A && git commit -m"Test"
# or
./vendor/bin/grumphp run

Result:

➜  nanospa git:(last_bits_on_spa) ✗ ./vendor/bin/grumphp run
GrumPHP is sniffing your code!

Running tasks with priority 0!
==============================

Running task 1/7: file_size... ✔
Running task 2/7: phpcs... ✔
Running task 3/7: git_branch_name... ✔
Running task 4/7: jsonlint... ✔
Running task 5/7: phplint... ✔
Running task 6/7: securitychecker_enlightn... ✔
Running task 7/7: composer... ✔
             ▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄
           ▄▄▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
         ▄▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
        ▐▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
        ▐▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
  ▄▐▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
 ▐▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
 ▐█▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
   ▀█▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▌
     ▀▀▓▓▓▓▓▓▓▓▓▓▓▓█▀▀▀▀▀▀▀▀▀▀▀▀▀▀████████████▄
      ▄████████▀▀▀▀▀                 ██████████
     ███████▀                         ██████▀
      ▐████      ██▌          ██       ████▌
        ▐█▌                            ███
         █▌           ▄▄ ▄▄           ▐███
        ███       ▄▄▄▄▄▄▄▄▄▄▄▄       ▐███
         ██▄ ▐███████████████████████████
        █▀█████████▌▀▀▀▀▀▀▀▀▀██████████▌▐
          ███████████▄▄▄▄▄▄▄███████████▌
         ▐█████████████████████████████
          █████████████████████████████
           ██ █████████████████████▐██▀
            ▀ ▐███████████████████▌ ▐▀
                ████▀████████▀▐███
                 ▀█▌  ▐█████  ▐█▌
                        ██▀   ▐▀
       _    _ _                         _ _
      / \  | | |   __ _  ___   ___   __| | |
     / _ \ | | |  / _` |/ _ \ / _ \ / _` | |
    / ___ \| | | | (_| | (_) | (_) | (_| |_|
   /_/   \_\_|_|  \__, |\___/ \___/ \__,_(_)
                  |___/

@veewee
Copy link
Contributor

veewee commented Jun 10, 2024

Hello,

It is not executed during grumphp run since it uses the git staged files to detect these keywords.
it only runs during pre-commit.

@cerw
Copy link
Author

cerw commented Jun 10, 2024

Hello,

It is not executed during grumphp run since it uses the git staged files to detect these keywords. it only runs during pre-commit.

Hello there,

Thanks for getting to me so fast, true ! but still not failing ?

GrumPHP detected a pre-commit command.
GrumPHP is sniffing your code!

Running tasks with priority 0!
==============================

Running task 1/8: file_size... ✔
Running task 2/8: phpcs... ✔
Running task 3/8: git_branch_name... ✔
Running task 4/8: jsonlint...
Running task 5/8: phplint... ✔
Running task 6/8: securitychecker_enlightn...
Running task 7/8: git_blacklist...
Running task 8/8: composer...
🤖 Skipping gptcommit because the githook isn't set up for the "Message" commit mode.
GrumPHP detected a commit-msg command.
GrumPHP is sniffing your code!

Running tasks with priority 0!
==============================

Running task 1/1: git_commit_message... ✔

Changes:

➜ nanospa git:(last_bits_on_spa) git diff b31aeafbf0a35c506cd4d3828113e7b7f192865b

─────────────────────────────────────────────────────────────────────────────────┐
app/Listeners/LogUserHasLoggedOff.php:29: public function handle(Logout $logout) │
─────────────────────────────────────────────────────────────────────────────────┘
29 ⋮ 29 │ // no user is logged in
30 ⋮ 30 │ return;
31 ⋮ 31 │ }
32 ⋮ │ dd(1);
⋮ 32 │ dd(2);
33 ⋮ 33 │ activity()
34 ⋮ 34 │ ->useLog('auth')
35 ⋮ 35 │ ->event('logout')

─────────────────────────┐
grumphp.yml:48: grumphp: │
─────────────────────────┘
48 ⋮ 48 │ - "dd("
49 ⋮ 49 │ - "var_dump("
50 ⋮ 50 │ - "exit;"
51 ⋮ │ # whitelist_patterns:
52 ⋮ │ # - add(
⋮ 51 │ whitelist_patterns:
⋮ 52 │ - add(
53 ⋮ 53 │ regexp_type: G
54 ⋮ │ match_word: true
⋮ 54 │ match_word: false
55 ⋮ 55 │ composer:


What has changed?


@veewee
Copy link
Contributor

veewee commented Jun 10, 2024

It might be so, that you need to apply keyword escaping, since ( is a reserved regex character.
See examples here:
https://github.com/phpro/grumphp/blob/v2.x/doc/tasks/git_blacklist.md

@cerw
Copy link
Author

cerw commented Jun 10, 2024

Hmm can not get it work, it runs but it has no tick?

image

Tried with " without, with match_word on and off..

grumphp:
  stop_on_failure: true
  process_timeout: 210
  parallel:
    enabled: true
    max_workers: 32
  tasks:
    file_size:
      max_size: 3M
      ignore_patterns: []
    phpcs:
      standard: PSR2
      warning_severity: 0
      error_severity: 1
      ignore_patterns:
        - ./tests
        - ./resources/*
        - ./database
        - ./bootstrap
        - ./public
        - ./config/
    git_branch_name:
      # whitelist:
      #   - '/([0-9]+)-/'
      #   - '/bugfix-*/'
      blacklist:
        - "/(shit|fuck|crap)/"
      # - "develop"
      # - "master"
    git_commit_message:
      allow_empty_message: false
      enforce_capitalized_subject: false
      enforce_no_subject_trailing_period: true
      enforce_single_lined_subject: false
      max_body_width: 400
      max_subject_width: 200
      case_insensitive: true
      multiline: true
      additional_modifiers: ""
    jsonlint: null
    phplint: null
    securitychecker_enlightn:
      lockfile: ./composer.lock
    git_blacklist:
      keywords:
        - die\\(
        - dump\\(
        - dd\\(
        - var_dump\\(
        - exit;
      whitelist_patterns:
        - add(
      regexp_type: G
      match_word: false
    composer:

@veewee
Copy link
Contributor

veewee commented Jun 14, 2024

Not sure what's going wrong there, it seems to be working on my specific setup.
Maybe you'dd rather use something like
https://github.com/phpro/grumphp/blob/v2.x/doc/tasks/phpparser.md

Or phpstan with https://github.com/ekino/phpstan-banned-code.

Both use PHP's AST to figure out if it's allowed or not which is less error prone on input like dump[space](
Whilst the blacklist task only checks for a regex match.

@veewee veewee added the bug label Jun 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants