Skip to content

Whitelist argparse and getopt as safe modules#142

Open
crowcreation wants to merge 1 commit into
ldayton:mainfrom
crowcreation:whitelist-argparse-getopt
Open

Whitelist argparse and getopt as safe modules#142
crowcreation wants to merge 1 commit into
ldayton:mainfrom
crowcreation:whitelist-argparse-getopt

Conversation

@crowcreation

Copy link
Copy Markdown

Summary

Move argparse and getopt from DANGEROUS_MODULES to SAFE_MODULES in the Python handler. Both are pure argv-parsing stdlib modules whose only side effects are stdout/stderr text and sys.exit on bad args — none of which is in the analyser's threat model:

  • print is already whitelisted, so --help/error text to stdout/stderr is consistent with existing policy.
  • sys.exit just terminates the script — not code execution, file I/O, or network access.
  • Neither module can read/write files, open sockets, spawn processes, or touch the filesystem.

Motivation

Nearly every real-world CLI script imports argparse. Listing it as dangerous caused the static analyser to ask for approval on essentially every script the handler sees — defeating the handler's purpose of auto-approving provably safe code. I hit this constantly with my own CLI tooling under scripts/, which prompted a dig through python.py.

What is deliberately NOT changed

  • logging — real FileHandler/SocketHandler/SMTPHandler exist
  • syssys.modules manipulation, sys.stdin/sys.stdout usable as file objects
  • getpass — reads from tty (I/O)
  • atexit — enables deferred execution of unreviewed handlers

These retain defensible I/O or exec vectors and stay in DANGEROUS_MODULES.

Test plan

  • Added test_safe_script_argparse_approved and test_safe_script_getopt_approved in TestPythonScriptAnalysis, mirroring existing safe-import test style.
  • Full tests/cli/test_python.py suite (127 tests) passes on Python 3.14 via uv run.
  • Existing dangerous-import tests (os, subprocess, pathlib, socket, requests, etc.) unaffected.

Both are pure argv-parsing stdlib modules. Their only side effects are
stdout (--help text), stderr (error messages), and sys.exit on bad args.
None of these is in the threat model:

- print is already whitelisted, so --help / error text to stdout/stderr
  is consistent with existing policy
- sys.exit just terminates the script; it is not code execution, file
  I/O, or network access
- Neither module can read or write files, open sockets, spawn processes,
  or access the filesystem

In practice, almost every legitimate CLI script imports argparse, so
listing it as dangerous caused the static analyser to ask for approval
on essentially every real-world script — defeating the handler's purpose
of auto-approving provably safe code.

Deliberately not changed: logging (real FileHandler), sys (sys.modules
manipulation, sys.stdin/stdout as file objects), getpass (reads from
tty), atexit (deferred exec of unreviewed handlers). These retain
defensible I/O or exec vectors.

Tests: adds argparse and getopt cases to TestPythonScriptAnalysis,
mirroring existing safe-import test style. Full python suite (127
tests) still passes.
@ldayton

ldayton commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Thanks for this — and the careful writeup. getopt is clean (pure string matching, no I/O), so that half is good to go.

argparse is trickier than "pure argv parsing", though: it has built-in file access that happens inside the library, so the script's AST looks innocent and the analyzer can't see it. A few that auto-approve once argparse is safe:

  • type=argparse.FileType('w'/'a') — opens/truncates a file
  • type=argparse.FileType('r') — reads any file
  • ArgumentParser(fromfile_prefix_chars='@') — reads a file named by an @path arg
  • type=eval — argparse calls eval(arg) at runtime (RCE)

The first three are the real issue: whitelisting argparse means an argparse script can no longer be proven safe by static analysis, which is the bar the handler sets ("if we can't prove it's safe, ask"). (type=eval is actually a separate, pre-existing gap — list(map(eval, data)) already analyzes as safe today with no argparse involved.)

Suggestion: land getopt as-is, and for argparse add targeted guards so the common case still auto-approves but FileType / fromfile_prefix_chars flag as dangerous. Happy to help wire that up if you'd like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants