Skip to content

Analyze python -c inline code (based on #134, alignment fixed) - #149

Merged
ldayton merged 3 commits into
mainfrom
lily/134-python-c-analysis
Jun 8, 2026
Merged

Analyze python -c inline code (based on #134, alignment fixed)#149
ldayton merged 3 commits into
mainfrom
lily/134-python-c-analysis

Conversation

@ldayton

@ldayton ldayton commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Statically analyze python -c inline code so safe one-liners auto-approve instead of always prompting, while anything containing a bash expansion falls back to ask.

This is @nickdavies's #134 (his commit is preserved here), rebased onto main now that its base #122 has merged, plus a fix for a token-alignment bug in the expansion guard and tests that cover it.

What #134 does

python -c was always ask. Now the inline body is run through the same AST safety analyzer used for script files: clean code → allow, dangerous imports/builtins → ask. Crucially, if the -c argument contains a bash expansion ($VAR, $(...), …) it's sent to ask, because bash rewrites the code at runtime and the static view is no longer trustworthy.

The bug this fixes

word_has_expansions was computed over the full word list but the python handler indexes it against the env-stripped tokens. So a leading assignment shifted every flag:

  • FOO=bar python -c "$VAR" → guard read the wrong token and didn't fire
  • wrapper commands (time/timeout/nice/…) dropped the flags entirely in the recursion

It only stayed safe because $ is invalid Python and the analyzer's syntax-error backstop caught it — i.e. the dedicated guard was silently a no-op in those cases. The fix re-slices the flags in lockstep with tokens at both the env-skip and the wrapper-recursion points (mirroring how the secrets PR handles opaque_positions).

Before vs after:

Command Before After
FOO=bar python -c "$VAR" ask (syntax backstop) ask (bash expansion)
time python -c "$VAR" ask (syntax backstop) ask (bash expansion)
FOO=bar python -c 'print(1)' allow allow

Tests

Added regression tests asserting the expansion reason fires (not the incidental syntax error) for env-prefixed and wrapper-wrapped -c, plus the safe-body-still-approves counterparts. Full suite: 10965 passing.

Out of scope

env X=1 python -c … routes through the env handler's delegation re-parse, a separate lossy round-trip tracked in #116; it remains safe via the syntax backstop and isn't touched here.

Closes #134.

Nick Davies and others added 3 commits June 8, 2026 14:07
- Statically analyze python -c inline code using the existing
  SafetyAnalyzer AST visitor. Safe code (no I/O, no dangerous imports)
  is auto-approved.
- Track per-word bash expansion flags (word_has_expansions) through
  HandlerContext so the python handler can detect when -c code contains
  $VAR or $(cmd) expansions and fall back to ask.
- Config module overrides (python-allow-module / python-deny-module)
  apply to inline code analysis as well.
word_has_expansions was built over the full word list but the python
handler indexes it against env-stripped tokens, so a leading FOO=bar
shifted every flag and the -c expansion guard read the wrong token.
Wrapper recursion (time/timeout/...) dropped the flags entirely.

Re-slice the flags in lockstep with tokens at both points. Without this
the guard silently no-ops and only the analyzer's syntax-error on $ saves
it; with it, FOO=bar python -c "$VAR" and time python -c "$VAR" report a
bash-expansion ask directly.

(The env handler's delegation path is a separate re-parse round-trip,
tracked in #116, and stays out of scope here.)
@ldayton
ldayton merged commit 88d80a4 into main Jun 8, 2026
1 check passed
@ldayton ldayton mentioned this pull request Jun 8, 2026
ldayton added a commit to nickdavies/Dippy that referenced this pull request Jun 8, 2026
ldayton#149 landed word_has_expansions (per-token: was the word built from a bash
expansion), which already does what this PR's opaque_positions needed and a
bit more — it also flags multi-expansion words like a$X$Y, and it's aligned
to the handler's tokens through env prefixes and wrapper recursion.

Resolve the conflict by dropping opaque_positions and pointing the kubectl
secret-exposure checks at word_has_expansions, so there's a single
expansion-tracking mechanism in the analyzer's security path instead of two
parallel ones. Behavior is equivalent-or-stricter; the kubectl tests are
unchanged and still pass.

Co-authored-by: Nick Davies <github@nicolasdavies.com.au>
ldayton added a commit that referenced this pull request Jun 8, 2026
kubectl get secret -o yaml / -o json / jsonpath and config view --raw expose
secret material, so they now require approval (while -o name / -o wide stay
allowed). Commands whose resource, subcommand, or output format comes from a
bash expansion are treated conservatively, since they could resolve to a
secret or a data-exposing format at runtime.

Opacity is detected via the per-token word_has_expansions flag (from #149)
rather than a separate opaque_positions set, keeping a single
expansion-tracking mechanism in the analyzer's security path. The kubectl
tests are unchanged and still pass.

Co-authored-by: Lily Dayton <43729618+ldayton@users.noreply.github.com>
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.

1 participant