Skip to content

Commit

Permalink
feat: move from bandit to ruff
Browse files Browse the repository at this point in the history
Ruff is an amazing linter that can replace bandit and more!

Also:

* fix failing tests (example.com has disappeared)
* make docker build depends on lint in CI
* update dependencies
  • Loading branch information
derlin committed Mar 18, 2023
1 parent ad1cfcb commit 39915b8
Show file tree
Hide file tree
Showing 14 changed files with 482 additions and 478 deletions.
10 changes: 6 additions & 4 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,21 @@ jobs:

- name: Check Style
run: |
poetry run black --diff --check rickroll
poetry run black --line-length 100 --diff --check rickroll
- name: Lint (ruff)
run: |
poetry run ruff rickroll
- name: Run Tests
run: |
poetry run pytest tests
- name: Check Common Vulnerabilities (SAST)
run: |
poetry run bandit -r rickroll
build:
if: ${{ !startsWith(github.event.head_commit.message, 'docs') }} # skip on documentation
uses: ./.github/workflows/reusable_docker-build-and-push.yaml
needs: [lint]
secrets: inherit # pass all secrets to the called workflow
with:
# publish only on push to main or develop (releases are handled in another workflow)
Expand Down
20 changes: 13 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,17 +103,23 @@ This eases collaboration and prevents big mistakes.
Each language has its own tools. This specific project (Python + Docker) uses:

* [black](https://github.com/psf/black) for (automatically) formatting python files,
* [bandit](https://bandit.readthedocs.io/) for checking vulnerabilities in python files,
* [~~bandit~~](https://bandit.readthedocs.io/) ~~for checking vulnerabilities in python files~~
* [ruff](https://beta.ruff.rs/docs/) for linting and checking style + vulnerabilities in python files,
* [checkov](https://checkov.io) for checking vulnerabilities in docker images.

black and bandit are listed under dev dependencies. To run the checks locally, use:
black and ruff are listed under dev dependencies. To run the checks locally:
```bash
# formatting
poetry run black rickroll # automatically fix the formatting issue, if possible
poetry run black --check --diff rickroll # only show the formatting issues
poetry run black --line-length 100 --check rickroll
poetry run ruff rickroll
```

# vulnerability scan
poetry run bandit rickroll
To fix the problems automatically (when possible), run:
```bash
# formatting
# automatically fix the formatting issue, if possible
poetry run black --line-length 100 --experimental-string-processing rickroll
# automatically fix the issues, if possible
poetry run ruff --fix rickroll
```

As checkov is quite heavy, it is run using a dedicated GitHub action in the CI.
Expand Down
733 changes: 356 additions & 377 deletions poetry.lock

Large diffs are not rendered by default.

22 changes: 20 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,27 @@ Flask-APScheduler = "^1.12"
pymongo = "^4.3.3"
flask-wtf = "^1.0.1"

[tool.poetry.dev-dependencies]
[tool.poetry.group.dev.dependencies]
black = "*"
bandit = "*"
pytest = "*"
mongomock = "^4"
ruff = "*"

[tool.ruff]
line-length = 100 # use black --line-length 100 --experimental-string-processing rickroll
# See https://beta.ruff.rs/docs/rules
select = [
"E", # pycodestyle error
"W", # pycodestyle warning
"F", # pyflakes
"A", # flakes8-builtins
"COM", # flakes8-commas
"C4", # flake8-comprehensions
"Q", # flake8-quotes
"SIM", # flake8-simplify
"PTH", # flake8-use-pathlib
"I", # isort
"N", # pep8 naming
"UP", # pyupgrade
"S", # bandit
]
42 changes: 20 additions & 22 deletions rickroll/__init__.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
from os import getenv, urandom
import urllib
from datetime import timedelta
from os import getenv, urandom

from werkzeug.exceptions import NotFound
from flask import Flask, request, render_template, redirect, url_for, flash
from validators.url import url as urlvalidate

from flask import Flask, request, render_template
from flask import Flask, flash, redirect, render_template, request, url_for
from flask_apscheduler import APScheduler
from flask_wtf import CSRFProtect
from flask_wtf.csrf import CSRFError
from flask_apscheduler import APScheduler
from validators.url import url as urlvalidate
from werkzeug.exceptions import NotFound

from .db import init_persistence, PersistenceException
from .rickroller import RickRoller, RickRollException
from .db import PersistenceError, init_persistence
from .rickroller import RickRoller, RickRollError


def create_app():

# == PARSE ENVIRONMENT
env_secret_key = getenv("APP_SECRET_KEY", urandom(10))
env_db_url = getenv("DATABASE_URL")
Expand All @@ -35,9 +32,7 @@ def create_app():
app.config["SERVER_NAME"] = server_name
app.logger.info(f"Using server name {server_name}")
else:
app.logger.warn(
"❗Running without the SERVER_NAME set may lead to errors in production"
)
app.logger.warning("❗Running without the SERVER_NAME set may lead to errors in production")

persistence = init_persistence(app, env_db_url, env_max_urls_per_user)

Expand All @@ -50,12 +45,15 @@ def create_app():
scheduler.init_app(app)
scheduler.start()
app.logger.info(
f"Registered cleanup job to run every {cleanup_interval} with retention {slug_retention}"
(
f"Registered cleanup job to run every {cleanup_interval} with retention"
f" {slug_retention}"
),
)

CSRFProtect().init_app(app)

safe_exceptions = [RickRollException, PersistenceException, CSRFError]
safe_exceptions = [RickRollError, PersistenceError, CSRFError]

@app.errorhandler(Exception)
def handle_exception(e):
Expand All @@ -77,7 +75,7 @@ def index():
if (url := request.form["url"]) is not None:
url = urllib.parse.unquote(url) # may be url-encoded
if not urlvalidate(url):
raise Exception(f"the provided URL is invalid.")
raise Exception("the provided URL is invalid.")
slug = persistence.get(url, client_ip())
redirects_after = 0
if "redirect_on_scroll" in request.form:
Expand All @@ -102,7 +100,7 @@ def rolled():

@scheduler.task("interval", id="del", **cleanup_interval)
def cleanup():
app.logger.info(f"Running cleanup.")
app.logger.info("Running cleanup.")
persistence.cleanup(retention=slug_retention)

@app.teardown_appcontext
Expand All @@ -111,11 +109,11 @@ def shutdown_session(exception=None):

@app.context_processor
def pass_global_flags_to_jinja_templates():
return dict(
cleanup_enabled=persistence.supports_cleanup,
retention=f"{env_slug_retention_value} {env_slug_retention_unit}",
scroll_redirects_after=env_scroll_redirects_after_default,
)
return {
"cleanup_enabled": persistence.supports_cleanup,
"retention": f"{env_slug_retention_value} {env_slug_retention_unit}",
"scroll_redirects_after": env_scroll_redirects_after_default,
}

def client_ip():
if (proxy_data := request.headers.get("X-Forwarded-For", None)) is not None:
Expand Down
5 changes: 3 additions & 2 deletions rickroll/__main__.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import argparse
from logging.config import dictConfig

from rickroll import create_app
from rickroll.log import logging_config
from logging.config import dictConfig


def main():
Expand All @@ -11,7 +12,7 @@ def main():

parser = argparse.ArgumentParser()
parser.add_argument("--port", default="8080")
parser.add_argument("--host", default="0.0.0.0") # nosec B104
parser.add_argument("--host", default="0.0.0.0") # noqa
parser.add_argument("-d", "--debug", action="store_true")
args = parser.parse_args()

Expand Down
2 changes: 1 addition & 1 deletion rickroll/db/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from .persistence import Persistence, NoPersistence, PersistenceException
from .persistence import NoPersistence, Persistence, PersistenceError # noqa


def init_persistence(app, connection_uri, max_urls_per_ip) -> Persistence:
Expand Down
8 changes: 4 additions & 4 deletions rickroll/db/mongo.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from datetime import timedelta
from typing import Self, Optional
from typing import Self

from pymongo import MongoClient

Expand All @@ -20,7 +20,7 @@ def __init__(self: Self, app, max_urls_per_ip, connection_uri):
super().__init__(app, max_urls_per_ip)
self.coll = self._get_mongo_collection(connection_uri)

def _get(self: Self, url: str) -> Optional[str]:
def _get(self: Self, url: str) -> str | None:
result = self.coll.find_one({self.__url: url})
return result[self.__slug] if result else None

Expand All @@ -35,7 +35,7 @@ def _create(self: Self, url: str, client_ip: str) -> str:
self.app.logger.debug(f"Inserted {tiny} in MongoDB.")
return tiny[self.__slug]

def _lookup(self: Self, slug: str) -> Optional[str]:
def _lookup(self: Self, slug: str) -> str | None:
if (tiny := self.coll.find_one({self.__slug: slug})) is not None:
return tiny["url"]
return None
Expand All @@ -55,7 +55,7 @@ def cleanup(self: Self, retention: timedelta):
if (cnt := self.coll.delete_many(query).deleted_count) > 0:
self.app.logger.info(f"Removed {cnt} record(s) from database.")

def teardown(self: Self, exception: Optional[Exception]):
def teardown(self: Self, exception: Exception | None):
pass

@classmethod
Expand Down
23 changes: 11 additions & 12 deletions rickroll/db/persistence.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import random
import string
from abc import ABC, abstractmethod
from typing import Self, Optional
from urllib.parse import quote, unquote
from datetime import datetime, timedelta
import random, string
from typing import Self
from urllib.parse import quote, unquote


class PersistenceException(Exception):
class PersistenceError(Exception):
def __init__(self, *args: object) -> None:
super().__init__(*args)

Expand All @@ -22,27 +23,25 @@ def get(self: Self, url: str, client_ip: str) -> str:
return result

if self.urls_per_ip(client_ip) >= self.max_urls_per_ip:
raise PersistenceException(
"Too many urls created. Please, try again later."
)
raise PersistenceError("Too many urls created. Please, try again later.")
return self._create(url, client_ip)

def lookup(self: Self, slug: str) -> str:
if (url := self._lookup(slug)) is not None:
self._update_time_accessed(slug)
return url
raise PersistenceException(f'Slug "{slug}" is invalid or has expired')
raise PersistenceError(f'Slug "{slug}" is invalid or has expired')

@abstractmethod
def _get(self: Self, url: str) -> Optional[str]:
def _get(self: Self, url: str) -> str | None:
pass

@abstractmethod
def _create(self: Self, url: str, client_ip: str) -> str:
pass

@abstractmethod
def _lookup(self: Self, slug: str) -> Optional[str]:
def _lookup(self: Self, slug: str) -> str | None:
pass

@abstractmethod
Expand All @@ -53,7 +52,7 @@ def _update_time_accessed(self: Self, slug: str):
def urls_per_ip(self: Self, ip: str) -> int:
pass

def teardown(self: Self, exception: Optional[Exception]):
def teardown(self: Self, exception: Exception | None):
pass

def cleanup(self: Self, retention: timedelta):
Expand All @@ -78,7 +77,7 @@ def _get(self: Self, url: str) -> str:
def _create(self: Self, url: str, client_ip: str) -> str:
return self._get(url)

def _lookup(self: Self, slug: str) -> Optional[str]:
def _lookup(self: Self, slug: str) -> str | None:
return unquote(slug)

def _update_time_accessed(self: Self, slug: str):
Expand Down
18 changes: 9 additions & 9 deletions rickroll/db/sql.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
from datetime import datetime, timedelta
from typing import Self, Optional
from datetime import timedelta
from typing import Self

from sqlalchemy import Column, String, DateTime
from sqlalchemy.orm import scoped_session, sessionmaker
from sqlalchemy import Column, DateTime, String
from sqlalchemy.engine import Engine
from sqlalchemy.ext.declarative import declarative_base
from sqlalchemy.orm import scoped_session, sessionmaker

from .persistence import Persistence

Expand Down Expand Up @@ -32,13 +32,13 @@ def __init__(self: Self, app, max_urls_per_ip, connection_uri):
super().__init__(app, max_urls_per_ip)
engine = self.__create_engine(connection_uri)
self.db_session = scoped_session(
sessionmaker(autocommit=False, autoflush=False, bind=engine)
sessionmaker(autocommit=False, autoflush=False, bind=engine),
)
Base.query = self.db_session.query_property()
Base.metadata.create_all(bind=engine)
self.app.logger.info(f"Initialized a SQL database.")
self.app.logger.info("Initialized a SQL database.")

def _get(self: Self, url: str) -> Optional[str]:
def _get(self: Self, url: str) -> str | None:
tiny = self.db_session.query(TinyUrl).filter(TinyUrl.url == url).first()
return tiny.slug if tiny else None

Expand All @@ -53,7 +53,7 @@ def _create(self: Self, url: str, client_ip: str) -> str:
self.app.logger.debug(f"Inserted {tiny} in SQL database.")
return tiny.slug

def _lookup(self: Self, slug: str) -> Optional[str]:
def _lookup(self: Self, slug: str) -> str | None:
if (tiny := self.db_session.query(TinyUrl).get(slug)) is not None:
return tiny.url
return None
Expand All @@ -73,7 +73,7 @@ def cleanup(self: Self, retention: timedelta):
self.app.logger.info(f"Removing {cnt} record(s) from database.")
query.delete()

def teardown(self: Self, exception: Optional[Exception]):
def teardown(self: Self, exception: Exception | None):
self.db_session.remove()

def __persist(self, tiny: TinyUrl):
Expand Down
2 changes: 1 addition & 1 deletion rickroll/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def logging_config(app_log_level):
"formatters": {
"default": {"format": "%(asctime)s %(levelname)8s | %(message)s"},
"app": {
"format": "%(asctime)s %(levelname)8s > %(message)s (%(filename)s:%(lineno)s)"
"format": "%(asctime)s %(levelname)8s > %(message)s (%(filename)s:%(lineno)s)",
},
},
"handlers": {
Expand Down
Loading

0 comments on commit 39915b8

Please sign in to comment.