Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions editors/vscode/samples/exhaustive.dippy
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,21 @@ deny-mcp mcp__*__delete_* "Block all MCP delete operations"
after-mcp mcp__github__create_* "Check the GitHub UI to verify"
after-mcp mcp__github__create_pull_request * "Share the PR URL with the team"

# -----------------------------------------------------------------------------
# ENVIRONMENT VARIABLE ALLOWLIST: allow-env
# -----------------------------------------------------------------------------

# A leading "VAR=value cmd" prefix is part of the literal command. By default
# only known-inert variables (locale, color, project name, ...) let a command
# inherit a bare "allow cmd" rule; a prefix that can change what runs
# (LD_PRELOAD=, GIT_SSH_COMMAND=, ...) still prompts. allow-env extends the
# inert set with project-specific variables.
allow-env COMPOSE_FILE
allow-env APP_DEBUG

# Glob patterns match by variable name
allow-env MYAPP_*

# -----------------------------------------------------------------------------
# AFTER RULES: PostToolUse feedback
# -----------------------------------------------------------------------------
Expand Down
5 changes: 5 additions & 0 deletions editors/vscode/snippets.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@
"body": "after-mcp ${1:mcp__github__create_*} \"${2:message}\"",
"description": "Post-action feedback for MCP tools"
},
"allow-env": {
"prefix": ["allow-env", "allowe"],
"body": "allow-env ${1:COMPOSE_FILE}",
"description": "Treat an env var prefix as inert when matching allow rules"
},
"set log": {
"prefix": ["set log", "log"],
"body": "set log ${1:~/.dippy/audit.log}",
Expand Down
2 changes: 1 addition & 1 deletion editors/vscode/syntaxes/dippy.tmLanguage.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"name": "comment.line.number-sign.dippy"
},
"rule": {
"begin": "^\\s*(allow-redirect|ask-redirect|deny-redirect|allow-mcp|ask-mcp|deny-mcp|after-mcp|allow|ask|deny|after|alias)\\b",
"begin": "^\\s*(allow-redirect|ask-redirect|deny-redirect|allow-mcp|ask-mcp|deny-mcp|after-mcp|allow-env|allow|ask|deny|after|alias)\\b",
"beginCaptures": {
"1": { "name": "keyword.control.dippy" }
},
Expand Down
31 changes: 31 additions & 0 deletions src/dippy/core/allowlists.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,3 +263,34 @@
"builtin", # run shell builtin
}
)


# === Inert Environment Variables ===
# Leading assignments of these variables don't change which executable runs
# or what it connects to, so "allow cmd" may match "VAR=value cmd". Anything
# that affects executable resolution, library loading, code execution, or
# connection targets (PATH, LD_*, BASH_ENV, PYTHON*, GIT_SSH_COMMAND,
# DOCKER_HOST, PAGER, EDITOR, ...) must never be added here.
# LC_* (LC_ALL, LC_CTYPE, ...) are locale settings, treated as inert via a
# prefix check at the call site rather than enumerated individually.

INERT_ENV_VARS = frozenset(
{
"APP_ENV", # framework environment name (Symfony)
"CI", # continuous-integration marker
"CLICOLOR", # BSD color toggle
"CLICOLOR_FORCE", # force color output
"COLUMNS", # terminal width hint
"COMPOSE_PROJECT_NAME", # docker compose project label
"FORCE_COLOR", # force color output
"LANG", # locale
"LANGUAGE", # locale fallback list
"LINES", # terminal height hint
"NODE_ENV", # node environment name
"NO_COLOR", # disable color output
"PYTHONUNBUFFERED", # unbuffered python stdio
"RAILS_ENV", # rails environment name
"TERM", # terminal type
"TZ", # timezone
}
)
68 changes: 58 additions & 10 deletions src/dippy/core/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@

from __future__ import annotations

import fnmatch
from dataclasses import dataclass, field
from pathlib import Path
from typing import Literal

from dippy.core.config import Config, match_redirect
from dippy.core.allowlists import SIMPLE_SAFE, WRAPPER_COMMANDS
from dippy.core.allowlists import INERT_ENV_VARS, SIMPLE_SAFE, WRAPPER_COMMANDS
from dippy.cli import get_handler, get_description, HandlerContext
from dippy.vendor.parable import parse, ParseError

Expand Down Expand Up @@ -387,6 +388,37 @@ def _analyze_redirects(
return decisions


def _config_match_decision(match, base: str) -> Decision | None:
"""Convert a config Match to a Decision, or None if no match."""
if not match:
return None
if match.decision == "allow":
return Decision("allow", f"{base} ({match.pattern})")
msg = match.message or match.pattern
return Decision(match.decision, f"{base}: {msg}")


def _env_assignments_inert(assignments: list[str], config: Config) -> bool:
"""True if every leading VAR=value assignment names an inert variable.

Inert means the assignment doesn't change which executable runs or what it
connects to, so the command may inherit a bare-command allow rule. A
variable qualifies if it is in INERT_ENV_VARS, is an LC_* locale setting,
or matches a user "allow-env" pattern.
"""
for assignment in assignments:
name = assignment.split("=", 1)[0]
if name in INERT_ENV_VARS or name.startswith("LC_"):
continue
# Env var names are case-sensitive; fnmatchcase avoids the platform
# case-folding that plain fnmatch (and so _glob_match) applies, which
# would make matching diverge between Linux and macOS.
if any(fnmatch.fnmatchcase(name, pattern) for pattern in config.env_vars):
continue
return False
return True


def _analyze_simple_command(
words: list[str],
config: Config,
Expand Down Expand Up @@ -418,15 +450,31 @@ def _analyze_simple_command(

cmd = SimpleCommand(words=words)
config_match = match_command(cmd, config, cwd, remote=remote)
if config_match:
if config_match.decision == "allow":
return Decision("allow", f"{base} ({config_match.pattern})")
elif config_match.decision == "deny":
msg = config_match.message or config_match.pattern
return Decision("deny", f"{base}: {msg}")
else: # ask
msg = config_match.message or config_match.pattern
return Decision("ask", f"{base}: {msg}")
decision = _config_match_decision(config_match, base)
if decision:
return decision

# 1.5. If no config match on raw tokens and the command carries leading
# environment variable assignments (FOO=bar cmd ...), retry matching with
# them stripped. Step 1 preserves explicit rules that include the
# assignment (e.g. "deny FOO=* cmd"). The retry is asymmetric:
# - deny/ask matches always apply (stripping only widens what is blocked),
# - an allow match applies only when every assignment names an inert
# variable, so a non-inert prefix (LD_PRELOAD=, GIT_SSH_COMMAND=, ...)
# can't ride in under a plain config "allow cmd" rule.
# Note: this gate covers the config-rule path only. The built-in safe-command,
# version/help, and wrapper paths below already strip env assignments before
# matching (pre-existing behavior), so a non-inert prefix on e.g. a SIMPLE_SAFE
# command is still allowed; tightening those is tracked as a follow-up.
if i > 0:
env_stripped_match = match_command(
SimpleCommand(words=tokens), config, cwd, remote=remote
)
decision = _config_match_decision(env_stripped_match, base)
if decision and (
decision.action != "allow" or _env_assignments_inert(words[:i], config)
):
return decision

# 2. Handle wrapper commands (time, timeout, etc.) - analyze inner command
if base in WRAPPER_COMMANDS and len(tokens) > 1:
Expand Down
41 changes: 41 additions & 0 deletions src/dippy/core/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,36 @@ def _parse_module_name(rest: str) -> str:
return mod


# Valid env var name or glob pattern: identifier characters plus the glob
# metacharacters * ? [ ] ! ^ -. Notably excludes '=', so "allow-env FOO=bar"
# (a value, not a name) is rejected -- matching is by variable name only.
_ENV_VAR_RE = re.compile(r"^[A-Za-z0-9_*?\[\]!^-]+$")


def _parse_env_var_pattern(rest: str) -> str:
"""Parse and validate an env var name or glob pattern from a directive arg.

Strips inline comments (# ...) and validates the pattern. Raises ValueError
if the name is missing, has extra words, or contains invalid characters.
"""
# Strip inline comments
if "#" in rest:
rest = rest[: rest.index("#")].rstrip()
if not rest:
raise ValueError("requires an env var name or pattern")
parts = rest.split()
if len(parts) != 1:
raise ValueError(f"requires exactly one env var name, got: {rest!r}")
name = parts[0]
if not _ENV_VAR_RE.match(name):
raise ValueError(f"invalid env var name or pattern: {name!r}")
if name in ("*", "**"):
# A catch-all would make every prefix inert -- including LD_PRELOAD and
# friends -- silently defeating the allowlist. Require a real name/prefix.
raise ValueError("'*' is too broad; name a variable or use a prefix glob")
return name


# Cache home directory at module load - fails fast if HOME is unset
_HOME = Path.home()

Expand Down Expand Up @@ -86,6 +116,10 @@ class Config:
aliases: dict[str, str] = field(default_factory=dict)
"""Command aliases mapping source to target (e.g., ~/bin/gh -> gh)."""

env_vars: list[str] = field(default_factory=list)
"""Env var names/patterns whose leading assignments may match a bare-command
allow rule (e.g. "allow-env COMPOSE_FILE")."""

python_allow_modules: list[str] = field(default_factory=list)
"""Extra modules to treat as safe for Python static analysis."""

Expand Down Expand Up @@ -152,6 +186,8 @@ def _merge_configs(base: Config, overlay: Config) -> Config:
after_mcp_rules=base.after_mcp_rules + overlay.after_mcp_rules,
# Aliases: overlay wins for conflicting keys
aliases={**base.aliases, **overlay.aliases},
# Env var allowlist accumulates
env_vars=base.env_vars + overlay.env_vars,
# Python module lists accumulate
python_allow_modules=base.python_allow_modules + overlay.python_allow_modules,
python_deny_modules=base.python_deny_modules + overlay.python_deny_modules,
Expand Down Expand Up @@ -242,6 +278,7 @@ def parse_config(text: str, source: str | None = None) -> Config:
mcp_rules: list[Rule] = []
after_mcp_rules: list[Rule] = []
aliases: dict[str, str] = {}
env_vars: list[str] = []
python_allow_modules: list[str] = []
python_deny_modules: list[str] = []
settings: dict[str, bool | int | str | Path] = {}
Expand Down Expand Up @@ -356,6 +393,9 @@ def parse_config(text: str, source: str | None = None) -> Config:
)
aliases[expanded_source] = alias_target

elif directive == "allow-env":
env_vars.append(_parse_env_var_pattern(rest))

elif directive == "python-allow-module":
mod = _parse_module_name(rest)
python_allow_modules.append(mod)
Expand All @@ -380,6 +420,7 @@ def parse_config(text: str, source: str | None = None) -> Config:
mcp_rules=mcp_rules,
after_mcp_rules=after_mcp_rules,
aliases=aliases,
env_vars=env_vars,
python_allow_modules=python_allow_modules,
python_deny_modules=python_deny_modules,
default=settings.get("default", "ask"),
Expand Down
114 changes: 113 additions & 1 deletion tests/test_analyzer_bugs.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,119 @@
import pytest

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


class TestConfigEnvVarPrefixMatching:
"""Config rules should match commands with leading env var assignments.

The config matcher checks raw tokens first, where "FOO=bar cmd" doesn't
match the pattern "cmd" because the assignment sits before the base
command. A second pass with env assignments stripped lets a bare-command
rule match "FOO=bar cmd" -- the same command the permission prompt
displays. The retry is asymmetric: deny/ask matches always apply (the
safe direction -- they only widen what is blocked), but an allow match
applies only when every leading assignment names an inert variable
(INERT_ENV_VARS, LC_*, or a user "allow-env" pattern). A non-inert prefix
like LD_PRELOAD= can change what actually runs, so it must not silently
inherit a plain "allow cmd".
"""

@pytest.fixture
def cwd(self):
return Path.cwd()

def test_allow_matches_env_prefixed_command(self, cwd):
"""allow symfony should match COMPOSE_PROJECT_NAME=x symfony ..."""
config = Config(rules=[Rule("allow", "symfony")])
result = analyze("COMPOSE_PROJECT_NAME=app symfony php foo.php", config, cwd)
assert result.action == "allow"

def test_allow_matches_without_env_prefix_unchanged(self, cwd):
"""Baseline: the same rule matches without an env prefix."""
config = Config(rules=[Rule("allow", "symfony")])
result = analyze("symfony php foo.php", config, cwd)
assert result.action == "allow"

def test_allow_matches_multiple_inert_env_prefixes(self, cwd):
"""Multiple inert leading assignments are stripped before re-matching."""
config = Config(rules=[Rule("allow", "symfony")])
result = analyze("CI=1 TZ=UTC symfony console cache:clear", config, cwd)
assert result.action == "allow"

def test_explicit_env_rule_still_wins_on_raw_pass(self, cwd):
"""A rule written against the raw tokens still matches in step 1."""
config = Config(rules=[Rule("deny", "FOO=* symfony *", message="nope")])
result = analyze("FOO=bar symfony php foo.php", config, cwd)
assert result.action == "deny"

def test_no_spurious_match_when_no_rule(self, cwd):
"""With no matching rule, an env-prefixed command still falls through."""
config = Config(rules=[Rule("allow", "rails")])
result = analyze("FOO=bar symfony php foo.php", config, cwd)
assert result.action == "ask"

def test_stripped_deny_rule_surfaces_via_retry(self, cwd):
"""A deny rule on the bare command applies to its env-prefixed form too.

The deny/ask retry is unconditional -- a non-inert prefix does not
exempt a command from a deny rule.
"""
config = Config(rules=[Rule("deny", "symfony", message="blocked")])
result = analyze("FOO=bar symfony php foo.php", config, cwd)
assert result.action == "deny"

def test_allow_blocked_for_non_inert_var(self, cwd):
"""An unknown env var must not inherit a bare allow rule."""
config = Config(rules=[Rule("allow", "symfony")])
result = analyze("FOO=bar symfony php foo.php", config, cwd)
assert result.action == "ask"

def test_allow_blocked_for_dangerous_var(self, cwd):
"""A non-inert prefix that can change execution does not inherit allow.

Uses "symfony" (no built-in handler and not in SIMPLE_SAFE) so the
config gate is the only thing that could allow the command -- a
SIMPLE_SAFE command like "ls" would be allowed by a later step
regardless, masking the gate.
"""
config = Config(rules=[Rule("allow", "symfony")])
result = analyze("LD_PRELOAD=/tmp/evil.so symfony php foo.php", config, cwd)
assert result.action == "ask"

def test_allow_env_config_extends_allowlist(self, cwd):
"""A user "allow-env" pattern lets that var inherit a bare allow rule."""
config = Config(rules=[Rule("allow", "symfony")], env_vars=["FOO"])
result = analyze("FOO=bar symfony php foo.php", config, cwd)
assert result.action == "allow"

def test_allow_env_glob_pattern(self, cwd):
"""An "allow-env" glob pattern matches by variable name."""
config = Config(rules=[Rule("allow", "symfony")], env_vars=["MYAPP_*"])
result = analyze("MYAPP_DEBUG=1 symfony php foo.php", config, cwd)
assert result.action == "allow"

def test_allow_env_glob_is_case_sensitive(self, cwd):
"""Env var names are case-sensitive on every platform.

A lowercase variable must not match an uppercase "allow-env" glob --
matching uses fnmatchcase so it does not case-fold on macOS/Windows.
"""
config = Config(rules=[Rule("allow", "symfony")], env_vars=["MYAPP_*"])
result = analyze("myapp_debug=1 symfony php foo.php", config, cwd)
assert result.action == "ask"

def test_mixed_inert_and_non_inert_blocks_allow(self, cwd):
"""One non-inert assignment poisons the whole prefix for allow."""
config = Config(rules=[Rule("allow", "symfony")])
result = analyze("CI=1 FOO=bar symfony php foo.php", config, cwd)
assert result.action == "ask"

def test_lc_prefix_is_inert(self, cwd):
"""LC_* locale variables are treated as inert via prefix match."""
config = Config(rules=[Rule("allow", "symfony")])
result = analyze("LC_ALL=C symfony php foo.php", config, cwd)
assert result.action == "allow"


class TestEnvVarPrefixHandling:
Expand Down
Loading