Fix config rules not matching env-var-prefixed commands

[powli]

Summary

Config rules like allow symfony fail to match commands with a leading environment-variable assignment such as COMPOSE_PROJECT_NAME=app symfony console cache:clear. Dippy displays this in the permission prompt as "symfony" (it strips the assignment for description/classification), but the config matcher checks the raw tokens — where COMPOSE_PROJECT_NAME=app sits before the base command and breaks the prefix match. So the user sees "symfony", writes allow symfony, and Dippy still asks.

This is the environment-variable variant of #136 (the case @Djaler raised in a comment: DOCKER_IMAGE=foo docker compose up not matching allow docker compose up).

Approach: inert-gated two-pass config matching

The first review (thanks @ldayton) flagged that blindly stripping FOO=bar for the allow retry breaks the literal-match guarantee — LD_PRELOAD=/tmp/evil.so cmd would inherit allow cmd. This revision implements the "known-inert subset" suggestion with a user escape hatch.

• Step 1 (unchanged): match the raw tokens. Preserves explicit rules that include the assignment, e.g. deny FOO=* symfony *.
• Step 1.5 (new): if step 1 finds no match and the command has leading env assignments, retry match_command against the env-stripped tokens. The retry is asymmetric:
  • a deny/ask match always applies — stripping only widens what gets blocked (safe direction);
  • an allow match applies only when every leading assignment names an inert variable. Otherwise it falls through to the default ask.

A variable is inert if it is in the curated default INERT_ENV_VARS (locale, color, project-name: COMPOSE_PROJECT_NAME, NODE_ENV, CI, TZ, LANG, TERM, …), is an LC_* locale setting, or matches a user-defined allow-env NAME directive (glob patterns matched case-sensitively; a bare * is rejected). Nothing affecting executable resolution, library loading, code execution, or connection targets (PATH, LD_*, BASH_ENV, PYTHON*, GIT_SSH_COMMAND, DOCKER_HOST, PAGER, EDITOR, …) is in the default set.

This fails closed: a variable nobody enumerated isn't inert, so the command falls back to ask — unlike a denylist of dangerous vars, which fails open on anything not listed. allow-env mirrors the existing python-allow-module directive shape.

from dippy.core.analyzer import analyze
from dippy.core.config import Config, Rule
from pathlib import Path

cfg = Config(rules=[Rule("allow", "symfony")])
analyze("COMPOSE_PROJECT_NAME=app symfony php x", cfg, Path("/tmp")).action  # allow (inert)
analyze("LD_PRELOAD=/tmp/evil.so symfony php x", cfg, Path("/tmp")).action   # ask   (non-inert)
analyze("FOO=bar symfony php x", cfg, Path("/tmp")).action                   # ask   (unknown)

cfg2 = Config(rules=[Rule("allow", "symfony")], env_vars=["FOO"])            # allow-env FOO
analyze("FOO=bar symfony php x", cfg2, Path("/tmp")).action                  # allow (opted in)

Scope / known limitation

This gate covers the config-rule path. The built-in safe-command (SIMPLE_SAFE), version/help, and transparent-wrapper paths already strip leading env assignments before matching (pre-existing behavior on main — e.g. LD_PRELOAD=… ls is already auto-allowed today), so a non-inert prefix on those still rides in. Tightening them with the same INERT_ENV_VARS gate is a natural follow-up; INERT_ENV_VARS lives in allowlists.py precisely so it can be reused there. Kept out of this PR to bound the scope to the reported bug.

Tests

TestConfigEnvVarPrefixMatching (analyzer) covers: inert allow matches; multiple inert prefixes; LC_* prefix; non-inert/unknown/dangerous prefixes blocked to ask; mixed inert+non-inert poisons the allow; allow-env extension and its case-sensitivity; deny retry unconditional; explicit raw-token rule wins in step 1. TestParseConfigEnvVars / TestMergeConfigsEnvVars cover directive parsing, value/* rejection, and scope merging. VS Code grammar, snippet, and sample updated. Full suite passes (11,054 tests).

[ldayton]

Thanks for splitting this out and mirroring #137's two-pass so the explicit deny FOO=* cmd form still matches first — that part is right.

My concern is that env assignments aren't inert the way a stray flag is, so stripping them before matching is riskier than the global-flags case. FOO=bar cmd is part of the literal command, and the prefix can change what actually executes:

• LD_PRELOAD=/tmp/evil.so ls would match allow ls
• DOCKER_HOST=tcp://other docker ps would match allow docker ps
• GIT_SSH_COMMAND='…' git fetch would match allow git fetch

The security model is explicit on this one: variables "stay literal … we match the literal string," expanded by the shell only after approval. Stripping the assignment to match allow cmd matches a string the shell will never run, and lets an unexamined prefix ride in under the allow. The two-pass preserves an explicit deny LD_PRELOAD=* *, but realistically nobody writes that, so the default outcome is that the assignment is ignored.

Rather than transparently stripping any FOO=bar prefix for the allow retry, could we hold the literal-match guarantee here — e.g. not strip env assignments for allow decisions at all, or only a known-inert subset? That keeps the env-prefixed command from silently inheriting a plain allow cmd.

[powli]

The concern about non-inert prefixes is fair — LD_PRELOAD=… cmd shouldn't silently inherit allow cmd, and the two-pass alone doesn't prevent that.

One data point worth factoring in, though: those examples are already auto-allowed on current main, without this PR:

from dippy.core.analyzer import analyze
from dippy.core.config import Config
from pathlib import Path

cfg = Config(rules=[])
analyze("LD_PRELOAD=/tmp/evil.so ls", cfg, Path("/tmp")).action         # allow (ls)
analyze("GIT_SSH_COMMAND=evil git fetch", cfg, Path("/tmp")).action     # allow (git fetch)
analyze("DOCKER_HOST=tcp://other docker ps", cfg, Path("/tmp")).action  # allow (docker ps)

_analyze_simple_command strips leading assignments before consulting SIMPLE_SAFE, wrapper commands, and the built-in handlers — only config rules currently see the raw tokens. So the literal-match guarantee is already not held on the built-in path; this PR as written would just have extended the existing behavior to config rules (including its hole).

Proposal: take your "known-inert subset" suggestion, with a user escape hatch.

1. A curated INERT_ENV_VARS frozenset in allowlists.py (alongside SIMPLE_SAFE / WRAPPER_COMMANDS): display/locale/project-naming vars only — COMPOSE_PROJECT_NAME, NODE_ENV, CI, TZ, LANG, LC_*, TERM, NO_COLOR, FORCE_COLOR, … Nothing that affects executable resolution, library loading, or connection targets.
2. A new config directive allow-env NAME (parallel to allow-redirect / allow-mcp) so users can extend it for vars their workflow needs — explicit, written-down opt-in, same trust model as any other allow rule.
3. The step-1.5 retry then becomes asymmetric:
   • a deny/ask match from the stripped retry always applies (stripping in that direction only widens what gets blocked),
   • an allow match is honored only when every leading assignment's variable name is in INERT_ENV_VARS ∪ user allow-env. Otherwise fall through to the default ask.

This fails closed: a variable nobody thought about isn't in the list, so the command falls back to ask — unlike a denylist of dangerous vars, which fails open on anything not enumerated. Your three examples all hit ask under an unmodified config, while the reported case (COMPOSE_PROJECT_NAME=app symfony … + allow symfony) works out of the box.

It also gives a migration path for the built-in-path behavior shown above: once the list exists, the same gate can be applied before SIMPLE_SAFE/handler classification in a follow-up, which would restore the literal-match guarantee everywhere rather than only for config rules. I'd keep that out of this PR to limit scope, but happy to file it.

If this direction works for you I'll update the PR: gate + directive + tests (parsing/merge in test_config.py, behavior in TestConfigEnvVarPrefixMatching), plus the VS Code grammar/snippets and a Configuration wiki blurb for allow-env.

[powli]

Implemented the inert-subset approach in 687cf0b (rebased onto current main, single commit). The allow retry no longer strips arbitrary prefixes:

• deny/ask retries are unconditional (stripping only ever blocks more);
• an allow retry applies only when every leading assignment names an inert variable — a curated INERT_ENV_VARS (locale/color/project-name), the LC_* family, or a user allow-env NAME opt-in. PATH, LD_*, BASH_ENV, GIT_SSH_COMMAND, DOCKER_HOST, PYTHON*, PAGER, EDITOR are deliberately excluded, so your three examples now resolve to ask under an unmodified config. It fails closed: an unenumerated variable is treated as non-inert.

allow-env mirrors the existing python-allow-module directive; a bare * is rejected so the allowlist can't be silently nullified, and globs match case-sensitively.

One honest scoping note: this gate is on the config-rule path only. The SIMPLE_SAFE / version-help / wrapper paths already strip env prefixes before matching on main (e.g. LD_PRELOAD=… ls is allowed today), so a non-inert prefix on those still rides in. I kept that out of this PR to stay on the reported bug, but INERT_ENV_VARS is in allowlists.py so the same gate can be applied there — happy to file a follow-up (or fold it in here if you'd prefer the guarantee to be uniform before merge).