Fourth review found the last import-path bypass: the is_dir() branch returned
None for a name whose dir has no __init__.py, without checking a same-name
sibling file. FileFinder loads a file over an init-less namespace dir, so an
attacker could overwrite a signed foo.py with malicious bytes, mkdir an empty
foo/ next to it (PROVENANCE untouched, still verifies), and any import of that
name ran the unverified foo.py - RCE with only plugins/ write access.
Fix: the dir-with-no-__init__.py branch no longer returns early; it falls
through to the leaf .py hash gate and the non-source refuse check. Invariant:
find_spec returns None for a plugins.* name ONLY where FileFinder would also
find nothing on the same __path__.
Everything else was confirmed sound this round: the owned plugins root, exec of
exact verified bytes (never .pyc/.so), the extension/bytecode refusal, plugin.py
read-once, the provenance signature gate, dev-exemption scoping, and #3/#4.
Symlink, suffix-ordering, cache-lifecycle, and loader-internal angles cleared.
2 regression tests (tampered .py + sibling dir; unsigned .py + sibling dir). All
13 bundled plugins still load under enforcement; 1067 pass, naming green.
Third review found the meta_path guard leaked exactly where it delegated to the
stdlib import system:
1. Non-.py submodules (CRITICAL). When a name had no dir and no .py, find_spec
returned None and the stdlib loaded a planted .so (ExtensionFileLoader) or a
sourceless .pyc unverified - an attacker deletes a signed .py and drops a
same-named .so with arbitrary init code, run on a normal request via core's
`from plugins.<name>.models import ...`. The guard now refuses any name for
which a non-source importable candidate (EXTENSION_SUFFIXES + BYTECODE_
SUFFIXES) exists on disk; None is reserved for genuinely-absent modules.
2. Top-level plugins/__init__.py (CRITICAL). It is in no plugin's provenance,
is attacker-writable, and Python runs it before any guarded submodule. The
guard now owns `plugins`: it execs an EMPTY package body (search points at
the plugins dir), so an overwritten plugins/__init__.py never runs.
Also: specs are built with spec_from_file_location so loaded modules get
__file__/__path__ (Flask blueprint root paths need it) while the loader still
execs the verified in-memory bytes - never re-reading the file.
Verified end to end: under PLUGIN_REQUIRE_SIGNED with all 13 bundled plugins
stamped, the app boots and loads every plugin through the guard; a tampered
plugin file is refused at load. 4 new guard tests (planted .so, sourceless
.pyc, absent-module defer, neutralized package root). Prior fixes #3/#4
confirmed still sound by the review. 1065 pass, naming green.
A re-review showed the previous "single import choke point" claim was wrong:
`plugins` is a normal importable package, so core request handlers that do
`from plugins.<name>.models import ...` never passed through the loader and ran
unverified - an attacker who dropped a file into plugins/<name>/ got arbitrary
in-process code execution on an ordinary HTTP request (and a planted .pyc ran
from cache). Gating load_plugin_class covered only plugin.py, one path of many.
Fix: importguard.py installs a sys.meta_path finder (under enforcement) that
intercepts EVERY plugins.<name>.* import, verifies the plugin's signed
provenance once, then verifies each module file against it and execs the exact
bytes it hashed - read once, compiled, exec'd, never a .pyc, never a re-opened
file. This closes the submodule bypass and the planted-bytecode read, and the
read-once exec closes the verify-vs-exec TOCTOU on the import path. The import
system, not one method, is the real choke point.
- init_app installs the guard when PLUGIN_REQUIRE_SIGNED, clears it otherwise.
- load_plugin_class now verifies plugin.py from a single read and execs that
buffer (finding #3 on that file); its submodule imports flow through the guard.
- docs: stamp-bundled must cover every plugin dir present (a disabled plugin's
module can be imported by core); recommend a read-only plugins/ owned by the
deploy user as defense in depth (closes the residual migrate-time race an
attacker with concurrent write could otherwise attempt).
Earlier review's fixes#3 (migrate code paths) and #4 (shelf content binding)
were confirmed sound and are unchanged. 7 import-guard tests (submodule verify,
tamper, unsigned refused, planted .pyc ignored, real import through the guard,
install/uninstall). 1061 pass, naming green.
