Plugins -Are they really reviewed?

Discussion in 'BukkitDev Information and Feedback' started by giffordj, Jun 19, 2013.

Thread Status:
Not open for further replies.
  1. Offline

    giffordj

    Normally I would sit back and not say anything, but I've run into a few things over the past few weeks, that has really ticked me off.

    I'm not going to name the plugins, but I think one basic test done by the reviewer is to see if the plugin even loads!! I have a had a few lately where plugin.yml file was misspelled or completely missing. Another one with illegal characters in the plugin.yml.

    I would also think that loading the plugin in test environment would be a basic check that would be done.

    Sorry for venting, but I just had 3 of them do that in a row.
     
  2. Offline

    Gravity

    Moved to the proper section.

    The BukkitDev Staff's job is to review plugins in order to ensure that there is no malicious code in it. We are unable to test every plugin to make sure they work because that would take up far too much time. Whenever we do happen to notice a potential issue with a plugin, a friendly staff member with a bit of extra time might let the author know about it outside of the review process. But the plugin will still be approved given that it's free of malicious code.

    If you would like the staff to review each plugin to make sure it works as intended, you might have to answer to people complaining about the already existing time it takes for plugins to be reviewed. This would add an infinite amount of time to that wait period; it's just not feasible. With plugins whose code doesn't actually do what they say they do at all, of course we're going to make them correct that should we catch it. But we cannot spend time loading massive plugins and testing every feature to make them work right.

    Saying "the DBO staff will not approve plugins that don't work right" sets a precedent that would force us to test every part of every plugin uploaded, even massive ones like WorldEdit, to ensure they work right. That's something we can't do. The staff are not quality control, we're here for your protection.
     
  3. Offline

    cdutka

    I find it will take me 2-3 weeks to bring an idea to code, to testing then to release; I also write plugins for my job for large enterprises so I am used to a long development cycle. If you see authors not testing you can either choose to encourage them to test more or choose not to use any of their plugins. Also check out the authors release history, you can see it from the file tabs I find plugins that I have issues with will have chunks of several file releases within a few days or even on the same day. This should raise a big flag that this user is coding, releasing as soon as they are inspired and probably are not putting the effort in for testing. Also look what is updated in there file release note, make sure there are details. Also look at comment and tickets, see if the author is getting a lot of complaints and if they are responding. With any sort of plugin/mod for any game QA will always be a problem the more you arm yourself with information the better your experience will be and if you're running a server the experience of your users will be.
     
  4. Offline

    giffordj

    I understand, but we are talking some basic things that need to be there for the plugin to work. If running a test on a severe is out of the question, could a simple script that contains basic checks be ran?

    Here's something I just put together really quick to do a basic check

    Code:
    #!/bin/bash
    #
    # Basic Plugin Check
    # --------------------------------------------------
    # Set Variables
    #
    JAR_FILES=$(ls -1 *.jar)
    PWD_DIR=$(pwd)
    TMP_DIR=/tmp
    #
    # Colors
    #
    BAD="\e[00;31m"
    GOOD="\e[00;32m"
    TEXT="\e[01;33m"
    NONE="\e[00m"
    #
    # --------------------------------------------------
    # Check if JAR is in our location
    #
    if [ "${JAR_FILES}" != "" ]; then
      for JAR_FILE in ${JAR_FILES}; do
        #
        # --------------------------------------------------
        #
        EXIT_VAR=0
        echo -e "${TEXT}Starting check on ${JAR_FILE}...${NONE}"
        #
        # --------------------------------------------------
        # Create temporary directory for testing
        #
        JAR_TMP=$(echo ${JAR_FILE} | sed -e 's/\.jar/.tmp/g')
        rm -rf ${TMP_DIR}/${JAR_TMP}
        install -d ${TMP_DIR}/${JAR_TMP}
        cd ${TMP_DIR}/${JAR_TMP}
        #
        # --------------------------------------------------
        #
        # Copy File Over
        cp ${PWD_DIR}/${JAR_FILE} ${TMP_DIR}/${JAR_TMP}
        #
        # --------------------------------------------------
        # Extract JAR
        #
        unzip -q ${JAR_FILE}
        #
        # --------------------------------------------------
        #
        # Plugin Checks Start Here
        #
        # --------------------------------------------------
        #
        # Check for the existance of plugin.yml
        #
        echo -e "${TEXT}  Checking for the existance of plugin.yml...${NONE}"
        CHECK_1=$(find * -name plugin.yml | grep -c plugin.yml)
        if [ "${CHECK_1}" != "0" ]; then
          echo -e "  ${GOOD}    plugin.yml exists.${NONE}"
        else
          echo -e "  ${BAD}    plugin.yml is missing.${NONE}"
          EXIT_VAR=255
        fi
        #
        # --------------------------------------------------
        #
        # Check for the existance of no ascii characters in
        # plugin.yml
        # --------------------------------------------------
        #
        for yml_file in `find * -name '*.yml'`; do
          echo -e "${TEXT}  Checking ${yml_file} for non-ASCII characters...${NONE}"
          rm -f ${JAR_TMP}/temp_file
          iconv -c -f utf-8 -t ascii ${yml_file} > ${TMP_DIR}/${JAR_TMP}/temp_file
          cmp -s ${yml_file} ${TMP_DIR}/${JAR_TMP}/temp_file
          CHECK_2=$?
          if [ "${CHECK_2}" = "0" ]; then
            echo -e "  ${GOOD}    ${yml_file} has only ASCII characters.${NONE}"
          else
            echo -e "  ${BAD}    ${yml_file} has non-ASCII characters.${NONE}"
            EXIT_VAR=255
          fi
        done
        #
        # --------------------------------------------------
        #
        # Plugin Checks End Here
        #
        if [ "${EXIT_VAR}" = "0" ]; then
          echo -e "${TEXT}Check of ${JAR_FILE} completed. ${GOOD}No Issues Found${TEXT}...${NONE}"
        else
          echo -e "${TEXT}Check of ${JAR_FILE} completed. ${BAD}1 or more issues Found${TEXT}...${NONE}"
        fi
      done
    else
      echo -e "${BAD}No JAR Files found.${NONE}"
    fi
    
    I've talked to several people over the past few weeks on this issue. I know I'm not a big fish out there, but a lot of people expect some basics to be checked, and the community assumes this is happening.

    Maybe I misunderstand what is going on in the review process. I'm a IT professional, and any code I write has to go through a series of scripts to make sure it has no basic errors. This process is automated and just spits an error out when i submit it up in our repository. If the code passes automation, or I contest the automation, then the formal review of the code happens.

    It should be easy to put a hook into the submission script, to do some basic checks and reject plugins automatically for silly mistakes that can be caught with simple scripting. Like a plugin missing plugin.yml.

    I hope others who feel this way will speak out as well. If some type of check script is added to the submission process, it could be eliminate a further review by an already overworked staff of volunteers. Granted automation doesn't always work, but at least it can scaled back to things that can be rejected automatically.

    Checking for malicious code is one thing, and the community appreciates the effort, but I think this needs to be taken a step further to prevent bad plugins from making it's way into the repository.

    Again, I don't mean any disrespect, just stating my opinion.

    EDIT by Moderator: merged posts, please use the edit button instead of double posting.
     
    Last edited by a moderator: Jun 2, 2016
  5. Offline

    TnT

    giffordj
    You present a great idea, in theory, but as Gravity stated, doing plugin QA is a slippery slope and sets an expectation that plugins will be bug free after getting approved by staff. We do not have the manpower to possibly undertake QA on the thousands of plugins we approve each month.
     
  6. Offline

    ZachBora

    I personaly only expect plugins to contain no malicious code. If it doesn't work, I'll advise the author.
    There should be 2 people checking if the plugin works : Author and users. If we add a third family (dev staff), that'll just be wasting time that could be used approving other plugins.

    Not let's pretend they do verify them, what does it give? They tell the author his plugin doesn't work. Author says it works on my machine just approve it! Bukkitdev staff time was wasted. Now if the bukkitdev staff doesn't verify if it works and just approves. Then we have 10 people downloading the plugin and telling the author it doesn't work. Now author will understand he made a mistake.

    Edit: Not to mention, a plugin could work for the Bukkitdev staff, but it won't work on another user because for example plugin was compiled with Java 1.7 instead of 1.6. There's many reasons a plugin could work for someone and not another. Bukkitdev staff can't test all possibilities. Which is what people would expect if they implemented such automatic scripts.
     
    Feaelin likes this.
  7. Offline

    Gravity

    The file review process is something we're not going to go into detail about because of its inherent nature. BukkitDev Staff review all plugins that are uploaded to our site extensively, but we cannot guarantee the result of a check will be anything other than a simple "to the best of our knowledge, this plugin is free of malicious code."

    This, of course, does not mean that the files are not looked at in greater detail, but the service that we provide to users is simply verification by a staff member that the code uploaded is benign; the details of that verification are not made public, just its result. With that in mind, we will take your suggestions into account, but unless we change the definition of a file 'approval' you should not assume that our checks do or do not contain any specific additional verification.
     
    Feaelin likes this.
  8. Offline

    giffordj

    If the plugin is checked into the repository a post-hook script could then instantly disapprove it without user intervention, if the script finds an issue. If there is no issue, it would then wait for the reviewer to review it for the malicious code checks.
     
  9. Offline

    ZachBora

    giffordj That can't happen any time soon. Changes to BukkitDev are done by Curse and there are many more urgent things that we've wanted for over a year that are still not present.
     
Thread Status:
Not open for further replies.

Share This Page