Merge 33ee4fd937 into 3c69dc68d2

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.

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 "

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),

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:

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)

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,

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

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)

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"]:

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"