parkpow / deep-license-plate-recognition

Automatic License Plate Recognition (ALPR) or Automatic Number Plate Recognition (ANPR) software that works with any camera.
https://platerecognizer.com/
MIT License
523 stars 122 forks source link

fix-error-usernameNXintegration #187

Closed ttayson closed 7 months ago

ttayson commented 7 months ago

Note: Sorry for the typo in the branch name.

For some reason, when using a username argument on Windows, the user account name is captured, interfering with the information passed to the script.

github-actions[bot] commented 7 months ago

Risk Level 2 - /home/runner/work/deep-license-plate-recognition/deep-license-plate-recognition/webhooks/Webhook_nx/main.py

The changes include the addition of command-line arguments for login credentials and environment variable handling. The risk is moderate due to the potential exposure of sensitive information if not handled properly. However, there are no API keys or secrets directly exposed in the code changes.

  1. Environment Variable Fallbacks: It's good practice to provide a way to set configuration through environment variables as a fallback. This is implemented correctly for server_host, args.login, args.password, args.ssl, args.debug, args.tag, and args.parkpow_token.

  2. Error Handling: The addition of a check to ensure server_host, args.password, and args.login are not None is a good practice to avoid runtime errors due to misconfiguration.

  3. Security Concern: The verify parameter in the session.post and session.get calls is controlled by a command-line argument which can disable SSL verification. This could lead to security vulnerabilities if SSL verification is turned off in a production environment. It is recommended to enforce SSL verification or provide clear warnings when it is disabled.

  4. Potential Resource Leak: The use of Timer to create recurring tasks can lead to resource leaks if not managed properly. Ensure that timers are canceled and cleaned up when the application is terminated or when they are no longer needed.

  5. Password Handling: Storing passwords in plain text or passing them through command-line arguments can be insecure. Consider using a more secure method of handling credentials, such as environment variables with proper access controls or a secret management tool.

  6. Boolean Argument Parsing: The --ssl argument is parsed as a string but is expected to be a boolean. This could lead to unexpected behavior. Use action='store_true' or action='store_false' for boolean flags.

Example for boolean argument parsing:

parser.add_argument(\"--ssl\", action='store_true', help=\"Enable SSL verification\")
  1. Logging Sensitive Information: Be cautious about logging sensitive information such as passwords or tokens. Ensure that any logging does not expose this information in logs.

๐Ÿ”’๐Ÿ›๐Ÿ‘€


Powered by Code Review GPT

ttayson commented 7 months ago

@adolfoarmas FYI

adolfoarmas commented 7 months ago

For some reason, when using a username argument on Windows, the user account name is captured, interfering with the information passed to the script.

@ttayson just curious, what value did you receive when "username" argument name was used? what a strange situation.

If it works now, LGTM!

ttayson commented 7 months ago

@adolfoarmas, in my case, I received "Talles Tayson" as the username from the login.

This happens because I use environment variables to capture the value when the user is using Docker, perhaps there is a better way to do this

adolfoarmas commented 7 months ago

@ttayson i see, there it is: https://adolfo-platerecognizer.tinytake.com/msc/OTM1NzU5OF8yMjkyNTQ1NQ

FYI https://docs.python.org/3/library/os.html#os.getenvb

os.getenv(key, default=None) Return the value of the environment variable key as a string if it exists, or default if it doesnโ€™t.

LGTM