Skip to content

← Config, Logs and Untrusted Input step 9 of 16

Hard End-to-End

Keep User Input Out of Commands and SQL

Your service runs a command-line tool for each request, and it looks up rows in a database by a key that the user sends. In both places, user input could turn into code. That bug is called injection. Text that was meant as data ends up read as part of a command or a query. Run a Command Without a Shell, earlier in this track, showed the fix for commands. You pass an argument list, so ; and $(...) arrive as plain text. In this exercise, you wrap that call with a timeout and your own exceptions. You also build a small SQL helper that keeps values out of the query text.

run_tool([sys.executable, "-c", "print('hi')"])  # "hi\n"
render_query("events")  # "SELECT value FROM events WHERE key = ?"
render_query("users")   # raises QueryError

Run a tool

Fill in run_tool(args, *, cwd=None, timeout=30.0). args is a non-empty list of strings. The first item is the program, and the rest are its arguments.

  • Call subprocess.run with list(args), check=True, capture_output=True, text=True, and the given timeout and cwd. Do not pass shell=True.
  • Return the captured stdout exactly, without stripping it.
  • Turn the two failures below into the supplied exceptions. Raise each one with raise ... from exc, so the original error stays attached as its cause.
subprocess.run raises You raise
CalledProcessError, because the program exited with a nonzero code ToolError(args, exc.returncode), with a note that holds the captured stderr
TimeoutExpired, because the program ran longer than timeout seconds ToolTimeout(args, timeout)

add_note attaches a line of text to an exception, and Python prints it below the error message in the traceback. Call it on the ToolError with a string that contains exc.stderr, so your exception keeps the tool's own error message. Let any other error pass through unchanged, such as the FileNotFoundError you get when the program does not exist.

On a timeout, subprocess.run kills the program and waits for it to exit before it raises. capture_output keeps all of the output in memory, so use this wrapper for tools with small output.

An argument list keeps the shell out, but the program still reads its own arguments. If you pass user text to an option that runs code, such as python -c or sh -c, the program runs that text. A value that starts with - can be read as an option, so put -- before user values when the tool supports it. Here the driver's code is trusted test code, and the strings in extra are the data.

Build SQL from trusted parts

Pasting a user's key into the query text has the same problem. A key such as x' OR '1'='1 becomes part of the query. The fix is parameter binding. You write a ? placeholder where a value goes and pass the values to the database driver separately, so a value can never change the query. A ? can stand for a value, but not for a table name. To choose a table, check the name against an allowlist, a fixed set of the names you accept.

Keep Sql = NewType("Sql", str). To mypy, Sql is a different type from str, so a function that takes Sql rejects a string built from user input. At runtime an Sql is a plain str, and Sql(text) checks nothing.

Fill in two functions:

  • render_query(table) accepts only "events" and "metrics", and raises QueryError for any other name. For an accepted name, it returns an Sql holding SELECT value FROM <table> WHERE key = ?, with the name in place of <table> and exactly that spacing.
  • execute(query, params) stands in for a database call. If query.count("?") differs from len(params), raise QueryError. Otherwise return f"{query} <<{len(params)}>>". Never put the parameter values into the string.

In real code, pass the query and the values separately to your database driver, as in cursor.execute(query, params) with sqlite3.

The supplied _negative_fixtures never runs. Its two calls pass a plain str to execute, and both must stay type errors. Strict mypy fails if their # type: ignore[arg-type] comments stop being needed, so keep the function unchanged.

What the driver reports

Keep solve as it is. It first sets sql to the result of execute(render_query(table), (key,)), or to "rejected" on a QueryError. Then it runs Python with the trusted code, passing the strings in extra as arguments, even when render_query rejected the table. It sets outcome to one of these:

  • "ok", and stdout holds what run_tool returned.
  • "failed", with returncode and stderr_in_notes. The second is true when a note contains "boom", the word the failing test programs write to stderr.
  • "timeout".

Only the "ok" report has a stdout key.

Going further

  • On a timeout, subprocess.run kills only the program it started. Any programs that program started can keep running.
  • On Windows, the system may run a .bat or .cmd file through a shell even without shell=True. Its arguments can then be read with shell rules.
  • Python's documentation covers subprocess security, NewType and SQLite placeholders.