From 33ee4fd937af4b4d7960c6f82bbddaff1b01b43b Mon Sep 17 00:00:00 2001 From: rivassec Date: Wed, 10 Jun 2026 20:07:36 -0700 Subject: [PATCH] Enable flake8-bandit (Ruff `S`) rule set Addresses the `# TODO: "S"` marker in pyproject.toml. Adds a per-file ignore for `pelican/tests/*` covering S-rules that are universally appropriate to suppress in test code (S101 assert, S105/S106 hardcoded test passwords, S108 /tmp paths, S311 non-cryptographic randomness, S603/S607 subprocess fixtures, S104 loopback bindings). Each remaining library finding gets a targeted `# noqa` with a justification comment explaining why the call is acceptable in context. Two `S101` suppressions in `pelican_import.py` and one `S504` in `server.py` flag latent issues that warrant follow-up but are intentionally deferred so this commit stays purely lint-enabling. No behavior changes. --- RELEASE.md | 16 +++++++++++++ pelican/cache.py | 6 ++++- pelican/generators.py | 6 ++++- pelican/server.py | 6 ++++- pelican/tools/pelican_import.py | 37 +++++++++++++++++++++++------ pelican/tools/pelican_quickstart.py | 5 +++- pelican/tools/pelican_themes.py | 5 +++- pelican/utils.py | 6 +++-- pelican/writers.py | 8 +++++-- pyproject.toml | 15 +++++++++++- 10 files changed, 93 insertions(+), 17 deletions(-) create mode 100644 RELEASE.md diff --git a/RELEASE.md b/RELEASE.md new file mode 100644 index 00000000..cd2dd169 --- /dev/null +++ b/RELEASE.md @@ -0,0 +1,16 @@ +Release type: patch + +Enable the `S` (flake8-bandit) Ruff rule set, addressing the `# TODO: "S"` +note in `pyproject.toml`. Adds a `pelican/tests/*` ignore for rules that +are universally appropriate to suppress in tests (asserts, hardcoded test +values, subprocess fixtures, loopback bindings) and adds inline `# noqa` +suppressions, each with a justification comment, on the existing library +findings. + +No behavior changes; this commit only enables the lint and documents why +each existing finding is acceptable. Two `S101` and one `S504` +suppressions in `pelican/tools/pelican_import.py` and `pelican/server.py` +flag latent issues that warrant follow-up (asserts used as control flow +or invariant checks are stripped under `python -O`; `ssl.wrap_socket` is +deprecated since Python 3.12) and are intentionally deferred to keep +this commit purely lint-enabling. diff --git a/pelican/cache.py b/pelican/cache.py index 8bd34268..1c3d7151 100644 --- a/pelican/cache.py +++ b/pelican/cache.py @@ -29,7 +29,11 @@ class FileDataCacher: if load_policy: try: with self._cache_open(self._cache_path, "rb") as fhandle: - self._cache = pickle.load(fhandle) + # The cache file is written by Pelican itself in the + # configured CACHE_PATH and is not user-supplied input. + # The risk of pickle load is bounded by the caller's + # control of CACHE_PATH. + self._cache = pickle.load(fhandle) # noqa: S301 except (OSError, UnicodeDecodeError) as err: logger.debug( "Cannot load cache %s (this is normal on first " diff --git a/pelican/generators.py b/pelican/generators.py index 515257c4..0a88b775 100644 --- a/pelican/generators.py +++ b/pelican/generators.py @@ -76,7 +76,11 @@ class Generator: os.path.join(simple_theme_path, "themes", "simple", "templates") ) - self.env = Environment( + # Pelican's intended behavior is to render whatever the theme author + # placed in their templates verbatim, including raw HTML in article + # content. Autoescape would break every existing theme. JINJA_ENVIRONMENT + # is exposed in settings so users who do want autoescape can enable it. + self.env = Environment( # noqa: S701 loader=ChoiceLoader( [ FileSystemLoader(self._templates_path), diff --git a/pelican/server.py b/pelican/server.py index 92b6ad76..2862406c 100644 --- a/pelican/server.py +++ b/pelican/server.py @@ -153,7 +153,11 @@ if __name__ == "__main__": args.path, (args.server, args.port), ComplexHTTPRequestHandler ) if args.ssl: - httpd.socket = ssl.wrap_socket( + # ssl.wrap_socket is deprecated since Python 3.12 in favor of + # ssl.SSLContext.wrap_socket, which would also let us pass a + # tightened protocol. Fixing this without breaking the local-dev + # workflow is tracked separately; suppressing for now. + httpd.socket = ssl.wrap_socket( # noqa: S504 httpd.socket, keyfile=args.key, certfile=args.cert, server_side=True ) except ssl.SSLError as e: diff --git a/pelican/tools/pelican_import.py b/pelican/tools/pelican_import.py index 1c03f9c9..3570fc9f 100755 --- a/pelican/tools/pelican_import.py +++ b/pelican/tools/pelican_import.py @@ -236,7 +236,12 @@ def blogger2fields(xml): continue try: - assert kind != "comment" + # Used as control flow: AssertionError is caught below and + # falls back to deriving filename from entry id. Replacing this + # with an explicit check is preferable but is a behavior change + # (asserts are stripped under `python -O`); deferred to a + # follow-up so this commit stays purely lint-enabling. + assert kind != "comment" # noqa: S101 filename = entry.find("link", {"rel": "alternate"})["href"] filename = os.path.splitext(os.path.basename(filename))[0] except (AssertionError, TypeError, KeyError): @@ -421,12 +426,14 @@ def dc2fields(file): def _get_tumblr_posts(api_key, blogname, offset=0): + # URL is constructed in this function from a literal scheme + a caller- + # supplied blogname argument; no user-controlled scheme is possible. url = ( f"https://api.tumblr.com/v2/blog/{blogname}.tumblr.com/" f"posts?api_key={api_key}&offset={offset}&filter=raw" ) - request = urllib_request.Request(url) - handle = urllib_request.urlopen(request) + request = urllib_request.Request(url) # noqa: S310 + handle = urllib_request.urlopen(request) # noqa: S310 posts = json.loads(handle.read().decode("utf-8")) return posts.get("response").get("posts") @@ -900,7 +907,12 @@ def download_attachments(output_path, urls): os.makedirs(full_path) print(f"downloading {filename}") try: - urlretrieve(url, os.path.join(full_path, filename)) + # urlretrieve is used by the WordPress importer to fetch + # attachment URLs that originate from a user-supplied WP export + # XML. The scheme is normalized above (`if scheme != "file"`) + # but the importer is intentionally lenient about source URLs + # so users can re-host content from arbitrary locations. + urlretrieve(url, os.path.join(full_path, filename)) # noqa: S310 locations[url] = os.path.join(localpath, filename) except (URLError, OSError) as e: # Python 2.7 throws an IOError rather Than URLError @@ -913,9 +925,10 @@ def is_pandoc_needed(in_markup): def get_pandoc_version(): + # cmd is a static literal list; no user input reaches subprocess here. cmd = ["pandoc", "--version"] try: - output = subprocess.check_output(cmd, text=True) + output = subprocess.check_output(cmd, text=True) # noqa: S603 except (subprocess.CalledProcessError, OSError) as e: logger.warning("Pandoc version unknown: %s", e) return () @@ -969,7 +982,11 @@ def fields2pelican( posts_require_pandoc.append(filename) slug = (not disable_slugs and filename) or None - assert slug is None or filename == os.path.basename(filename), ( + # Programming invariant from the upstream WordPress XML parser; would + # be more robust as an explicit raise (asserts are stripped under + # `python -O`), but converting it is a behavior change deferred to + # a follow-up. + assert slug is None or filename == os.path.basename(filename), ( # noqa: S101 f"filename is not a basename: {filename}" ) @@ -1064,7 +1081,13 @@ def fields2pelican( ) try: - rc = subprocess.call(cmd, shell=True) + # cmd is composed from `out_filename` and `html_filename`, + # both Pelican-controlled paths derived from the importer's + # output_path argument. shell=True is needed for the + # legacy invocation; this is a CLI tool the user runs + # against their own export data, not a web-facing surface. + # Removing shell=True is a behavior change and is deferred. + rc = subprocess.call(cmd, shell=True) # noqa: S602 if rc < 0: error = f"Child was terminated by signal {-rc}" sys.exit(error) diff --git a/pelican/tools/pelican_quickstart.py b/pelican/tools/pelican_quickstart.py index 77657162..4448a6c3 100755 --- a/pelican/tools/pelican_quickstart.py +++ b/pelican/tools/pelican_quickstart.py @@ -41,7 +41,10 @@ else: _DEFAULT_LANGUAGE = _DEFAULT_LANGUAGE.split("_")[0] _TEMPLATES_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "templates") -_jinja_env = Environment( +# Quickstart renders Pelican-shipped templates (no user input) into a new +# project's settings file. Autoescape would mangle the resulting Python / +# Makefile / RST output. +_jinja_env = Environment( # noqa: S701 loader=FileSystemLoader(_TEMPLATES_DIR), trim_blocks=True, keep_trailing_newline=True, diff --git a/pelican/tools/pelican_themes.py b/pelican/tools/pelican_themes.py index 29fd7a30..f1947009 100755 --- a/pelican/tools/pelican_themes.py +++ b/pelican/tools/pelican_themes.py @@ -231,7 +231,10 @@ def install(path, v=False, u=False): for root, dirs, files in os.walk(theme_path): for d in dirs: dname = os.path.join(root, d) - os.chmod(dname, 493) # 0o755 + # Theme directories must be world-readable + # and traversable when served by web servers + # running under non-owner uids. + os.chmod(dname, 493) # 0o755 # noqa: S103 for f in files: fname = os.path.join(root, f) os.chmod(fname, 420) # 0o644 diff --git a/pelican/utils.py b/pelican/utils.py index 90d1a3fd..d32e5ec8 100644 --- a/pelican/utils.py +++ b/pelican/utils.py @@ -270,8 +270,10 @@ def slugify( # see: https://en.wikipedia.org/wiki/Unicode_equivalence return unicodedata.normalize("NFKC", text) - # strip tags from value - value = Markup(value).striptags() + # strip tags from value. Markup() here is used purely as a vehicle for + # striptags() on text we are about to slugify; the result is plain text + # and is never rendered as HTML. + value = Markup(value).striptags() # noqa: S704 # normalization value = normalize_unicode(value) diff --git a/pelican/writers.py b/pelican/writers.py index 67d52ecd..81145ae1 100644 --- a/pelican/writers.py +++ b/pelican/writers.py @@ -44,8 +44,10 @@ class Writer: feed_title = context["SITENAME"] + " - " + feed_title else: feed_title = context["SITENAME"] + # Markup() here serves as a wrapper for striptags(); the resulting + # plain text is fed to the feed library, not rendered as HTML. return feed_class( - title=Markup(feed_title).striptags(), + title=Markup(feed_title).striptags(), # noqa: S704 link=(self.site_url + "/"), feed_url=self.feed_url, description=context.get("SITESUBTITLE", ""), @@ -53,7 +55,9 @@ class Writer: ) def _add_item_to_the_feed(self, feed, item): - title = Markup(item.title).striptags() + # Same pattern as above: Markup is used to call striptags(); the + # result is plain text consumed by feedgenerator, not HTML output. + title = Markup(item.title).striptags() # noqa: S704 link = self.urljoiner(self.site_url, item.url) if self.settings["FEED_APPEND_REF"]: diff --git a/pyproject.toml b/pyproject.toml index 3e914e84..5cfb9197 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -161,7 +161,7 @@ select = [ "TCH", # flake8-type-checking "T10", # flake8-debugger "T20", # flake8-print - # TODO: "S", # flake8-bandit + "S", # flake8-bandit "YTT", # flake8-2020 # TODO: add more flake8 rules ] @@ -201,6 +201,19 @@ ignore = [ # allow imports after a call to a function, see the file "E402" ] +"pelican/tests/*" = [ + # asserts are how tests assert; suppress flake8-bandit's blanket S101 + "S101", + # tests legitimately use hardcoded values for fixtures, sample data, + # subprocess fixtures, partial paths, etc. + "S105", # hardcoded password string + "S106", # hardcoded password func arg + "S108", # hardcoded /tmp paths in tests + "S311", # standard pseudo-random not safe; tests don't need cryptographic randomness + "S603", # subprocess without shell=True; tests intentionally invoke binaries + "S607", # start-process-with-partial-path; tests rely on PATH lookup + "S104", # bind-all-interfaces; tests bind to 0.0.0.0 inside loopback fixtures +] "pelican/tests/test_utils.py" = [ # the tests have a bunch of unicode characters "RUF001"