Feature: Flexible command restrictions#6
Conversation
dbernheisel
left a comment
There was a problem hiding this comment.
Most of this looks to include changes from the other branch -- ignoring that, I think this is a good direction, though let's expand the CommandPolicy idea a bit into a struct that can be built to hold more decisions.
I'm thinking something like:
%CommandPolicy{
commands: :no_external | [{:allow | :disallow, [...]}, fn x -> end] | fn x -> ... end,
paths: [~r/.../, "..", fn x -> ... end],
files: [~r/.../, "..", fn x -> ... end],
}this would be evaluated at the time of the access request and respond appropriately.
eaa809f to
c62d70a
Compare
Move command restriction enforcement from CommandPort to AST level.
CommandPort is now a plain process-spawning pipe with no policy logic.
Three policy types:
- :unrestricted (default, no restrictions)
- :disallow_external (blocks all external commands)
- {:allow, MapSet} (whitelist specific commands)
Policy is checked in AST.Command.resolve_and_execute, Pipeline.external_command?,
and each builtin that dispatches externally (exec, command, coproc). Background
jobs also check policy before spawning. Policy is immutable once set -- cannot be
changed via set or shopt at runtime.
Backwards compatible: `restricted: true` normalizes to `:disallow_external`.
Normalize {:allow, list} to {:allow, MapSet} at session init so users
can write {:allow, ["cat", "grep"]} instead of wrapping in MapSet.new.
c62d70a to
3892fbe
Compare
Replace the union type (`@type t :: :unrestricted | :disallow_external |
{:allow, MapSet.t()}`) with a `%CommandPolicy{}` struct supporting three
dimensions: commands, paths, and files.
The commands field now supports:
- `:unrestricted` / `:no_external` atoms
- Rule lists with `{:allow, items}`, `{:disallow, items}`, and `fn/1`
- Items can be strings (exact + basename match), Regex, or `:all` catch-all
- Single function `(String.t() -> boolean())` for dynamic evaluation
Rules evaluate in order — first match wins, deny by default.
Structural changes:
- Promoted from `state.options.command_policy` to top-level `state.command_policy`
- Immutability now automatic (set builtin only touches options map)
- `paths` and `files` fields reserved for future filesystem policy
- Removed all legacy/backwards-compat code (restricted: true, :disallow_external)
Cover all enforcement points across all policy types:
- Coproc restriction (was completely untested)
- Exec with allowlist, denylist, and function policies
- Background jobs with allowlist, denylist, and function policies
- Pipeline with denylist, function, and regex policies
- command -v with denylist and function policies
- Session inheritance (struct form, pre-built struct)
Add struct edge case tests:
- Empty rule list, empty allow/disallow items
- {:allow, :all} as standalone policy
- Multiple disallow rules in sequence
- Regex full-path matching
- from_state with non-struct and nil values
- new/1 from map, function that always rejects
69 → 98 test cases.
|
Hey @dbernheisel, that's a great insight on moving the data structure to a struct and planning for support across different dimensions (commands, paths, files). I've reworked %CommandPolicy{
commands: :unrestricted | :no_external | [rules] | fn/1,
paths: nil, # reserved for future filesystem policy
files: nil # reserved for future filesystem policy
}Rules support strings, regex, and functions — evaluated in order, first match wins: # Denylist with catch-all allow
Bash.Session.new(command_policy: [commands: [{:disallow, ["rm", "dd"]}, {:allow, :all}]])
# Regex-based
Bash.Session.new(command_policy: [commands: [{:allow, [~r/^git-/]}]])
# Function-based
Bash.Session.new(command_policy: [commands: fn cmd -> String.starts_with?(cmd, "safe-") end])Also promoted The |
…erop Policy rules can now gate all command categories, not just externals. Category atoms (:builtins, :externals, :functions, :interop) work as rule items alongside strings and regexes. The command dispatch in Command AST now resolves category first, then checks policy, then executes. Backwards compatible: check_command/2 defaults to :external, :no_external still only blocks externals, and fun/1 only fires for externals.
Add a Common Recipes table to the moduledoc showing how to configure every practical category combination. Expand integration tests from 8 to 30 with systematic describe blocks covering each single-category policy, multi-category combinations, disallow-by-category, disallow-by- name, fun/2, and block-everything.
Motivation
PR #5 consolidated all
ExCmdusage intoCommandPortand added restriction gates there. However, per @dbernheisel's review feedback, restriction decisions should be made at a higher level — not in the process-spawning layer. The booleanrestricted: true/falseis also too rigid; users need fine-grained control over which commands are allowed.Design
Bash.CommandPolicystructAn extensible struct stored on
Bash.Sessionstate, immutable once set:Category-aware policy engine
Every command falls into one of four categories, and rules can gate any of them:
:builtin:builtinsecho,cd,export, etc.:external:externals$PATH:function:functionsname() { ... }:interop:interopdefbashCommon recipes
commands: :unrestrictedcommands: :no_externalcommands: [{:allow, [:builtins]}]commands: [{:allow, [:builtins, :functions]}]commands: [{:allow, [:builtins, "cat", "grep"]}]commands: [{:disallow, [:externals]}, {:allow, :all}]commands: [{:disallow, ["eval", "source"]}, {:allow, :all}]commands: [{:disallow, :all}]Rule evaluation
Rules are evaluated in order, first match wins, fail-closed (no match = deny):
{:allow, items}/{:disallow, items}— items can be strings, regexes, or category atoms{:allow, :all}/{:disallow, :all}— catch-allfun/2— receives(name, category), returntrueto allowfun/1— legacy, only evaluated for:externalcommandsCommand dispatch restructured
resolve_and_executeinAST.Commandnow follows resolve-then-check-then-dispatch::function,:interop,:builtin, or:external)CommandPolicy.check_command(policy, name, category)Policy enforcement points
AST.Command.resolve_and_executePipeline.external_command?:external(streaming optimization)execbuiltin:externalcommandbuiltin:external(run and lookup)coprocbuiltin:externalSession(background jobs):externalBackwards compatible
check_command/2defaults to:externalcategory:no_externalonly blocks externals (builtins/functions/interop pass through)fun/1policies only fire for externalsoptions: %{restricted: true}normalized tocommand_policy: :no_externalImmutable once set
setbuiltin preservescommand_policyacross option changesshopt restricted_shellis read-only, reflectscommand_policy != :unrestrictedUsage
Test plan
test/bash/command_restrictions_test.exs— 142 tests covering::no_externalblocks external commands (simple, absolute path, pipeline, subshell, command substitution, background jobs){:disallow, :all}blocks everythingfun/2receives category,fun/1backwards compat (external-only)setandshoptcommand -vrespects policyexec,coproc, background jobs, pipelines respect all policy typesCommandPolicystruct: normalization,check_command/3,command_allowed?/3mix format --check-formattedcleanmix compile --warnings-as-errorsclean