Skip to content

← The Edge of the System step 3 of 12

Hard End-to-End

Injection: the list form, and a SQL type your checker enforces

os.system(f"convert {filename} out.png")

With filename = "a.png; rm -rf ~" that is remote code execution, and it is still being written today. The fix is not “remember to quote”. The fix is a boundary at which injection is structurally impossible.

The list form is not a style preference

From the subprocess documentation:

this library will not implicitly choose to call a system shell… If the shell is invoked explicitly, via shell=True, it is the application’s responsibility to ensure that all whitespace and metacharacters are quoted.

When you pass a list, the argument vector goes to execve directly. There is no parser between you and the kernel, so ;, &&, $(...), backticks and globs are characters in a string. Not “escaped” — never interpreted in the first place. That is a categorical difference, and it is why the rule is worth being dogmatic about.

One caveat to carry: on Windows, .bat and .cmd are launched through a shell regardless of how you pass the arguments. The list form protects you everywhere else.

What you write

def run_tool(args: Sequence[str], *, cwd: Path | None = None,
             timeout: float = 30.0) -> str: ...

Every keyword earns its place:

  • check=True — without it a non-zero exit sails straight past you. Your caller gets an empty string and believes the tool succeeded. This is the most common subprocess bug that is not injection.
  • capture_output=True — you need stderr to produce a diagnosable error.
  • text=True — decode once, here, at the boundary.
  • timeout= — a child with no budget is a hang, and a hang is an outage.

Translate the stdlib exceptions into your domain:

stdlib yours
subprocess.CalledProcessError ToolError, .returncode set, stderr attached with add_note()
subprocess.TimeoutExpired ToolTimeout

Chain both with raise ... from exc. add_note() (PEP 678) is the right home for stderr: it appears in the traceback, it does not bloat the message, and it survives being collected into an ExceptionGroup.

The typing that bites

subprocess.run is one of the most heavily overloaded functions in typeshed, and the overloads encode real behaviour:

  • text=True gives CompletedProcess[str]; text=False gives CompletedProcess[bytes]. Getting this wrong is a TypeError at runtime and an error at check time.
  • capture_output=False types .stdout as None, so result.stdout.strip() is a --strict error. The checker is telling you that you forgot to capture.

The SQL half — and an honest note about LiteralString

PEP 675 added LiteralString precisely for this: a parameter typed LiteralString accepts string literals and concatenations of literals, but not a string built from runtime data, so "... WHERE k = '" + user + "'" is a type error. It is the textbook answer, and pyright implements it.

mypy 2.3 does not implement PEP 675 — it treats LiteralString as a plain str and the call above passes. Knowing which of your gates actually enforces a rule is part of the job; a defence your CI does not check is a comment.

So here you build the defence that mypy does enforce — the tainted/trusted split from NewType:

Sql = NewType("Sql", str)

def render_query(table: str) -> Sql: ...          # the only minting site
def execute(query: Sql, params: Sequence[object]) -> str: ...

Sql is a str at runtime with zero overhead, and a distinct type statically. Because render_query is the only function that returns one, every value that reaches execute provably came through the allowlist. Handing execute a concatenated string is an arg-type error — and the module’s _negative_fixtures proves it, because --strict includes --warn-unused-ignores: if you weaken Sql to str, the two # type: ignore comments become unused and the submission fails to type-check.

Why an allowlist for the table but parameters for the value? Because an identifier cannot be parameterised. WHERE key = ? is what the driver is for; FROM ? is not valid SQL in any dialect. When you must interpolate an identifier, the only safe source is a closed set you wrote down.

execute here is a stand-in for a real cursor: it checks that the placeholder count matches the parameter count and returns f"{query} <<{len(params)}>>".

What the tests do

They invoke sys.executable -c "<code>" — portable, no filesystem, no network — and check three things: a non-zero exit raises with stderr recoverable from the notes; an argument containing ; echo pwned arrives at the child literally and nothing executes it; and a child that sleeps for a minute is killed at its deadline rather than hanging the caller.