From: Mike Lowis Date: Fri, 2 Oct 2026 18:18:50 +0000 (-0400) Subject: added shell command execution X-Git-Url: https://git.mdlowis.com/?a=commitdiff_plain;p=proto%2Fnlshell.git added shell command execution --- diff --git a/nl-shell-harness-spec.md b/nl-shell-harness-spec.md index d588d0b..128d8fd 100644 --- a/nl-shell-harness-spec.md +++ b/nl-shell-harness-spec.md @@ -87,7 +87,7 @@ Raw input becomes an array of typed tokens via a single-pass lexer (`Lexer.token Literals are never embedded back into a string with markers — they're carried as token data, so nothing downstream can accidentally lowercase, split, or "correct" a filename, and no restore/unmasking step is needed. `token.key` returns the word text or the literal's **kind** (not its value), so `delete old.log` and `delete report.pdf` normalize to the identical match string `delete file` for Tier 1/2 comparison, while `token.text` on a literal still holds the real value for execution. -Known false-positive guards: a bare `?` is never a glob character, so it always ends a question. (The `e.g.`/`i.e.` abbreviation guard described in the original source listing below is **not present** in the current `nlshell.rb` implementation — a gap to revisit.) +Known false-positive guards: a bare `?` is never a glob character, so it always ends a question. The `e.g.`/`i.e.` abbreviation guard described in the original source listing below is now present in `nlshell.rb`: the file-literal path strips trailing sentence punctuation first, then checks the remaining text against `ABBREVIATIONS`, consuming one more `.` as the abbreviation's own (so it reads as a `:word`, not a sentence-ending `punct`). Full source (drop-in, no dependencies beyond Ruby's `strscan` stdlib): @@ -340,10 +340,20 @@ Collapses the former `Classifier` pre-filter for `conditional`/`loop` into the s 5. **Executor:** `Executor.call` currently flattens every arg via `.text`; it needs a branch so a `:body`/`:then`/`:else` arg executes its nested nodes recursively instead of being passed as a string. **Implemented**: `Executor.call` passes non-`Token` args through unchanged, and a new `Executor.run(node)` is the shared permission-checked entry point (confirms `:ask` tools, then calls) — used by both the REPL loop and the `conditional` tool's own action, so a destructive body still requires its own confirmation regardless of nesting. 6. **Limits:** the depth cap (§7, 3) and loop iteration cap (§10, 20) must be enforced in this new recursive path, not just documented. **Not implemented** — nesting depth is currently unbounded in practice (e.g. `if A exists do if B exists do archive B` resolves fine), and there is no loop tool yet to iterate. -**Known gap, discovered during implementation — a connective word can't double as rule syntax.** `builtins`' `connectives` list (§5.5) makes `"then"` an unconditional clause separator at the `Lexer.clauses` stage. A rule pattern like `"if :target exists then :body"` can never match, because clause-splitting tears it into two clauses before Tier1/Tier2 ever see the full span — the pattern's literal `"then"` and the lexer's connective `"then"` collide. Worked around for the minimal slice by using `"do"` as the condition/body joiner instead (`"if :target exists do :body"`), but this is a generalizable problem: any future control-flow tool that wants a connective word (`"then"`, `"and"`, `"next"`, etc.) as literal syntax will hit the same wall, and loop syntax (`"for each :item in :list do :body"` or similar) needs to be designed around it too. Needs a real fix before generalizing past the minimal slice — e.g. scoping connective-splitting to happen only outside a span a control-flow tool's pattern would claim, or reserving a connective-free joiner vocabulary for rule authors. +**Resolved gap, discovered during implementation — a connective word can't double as rule syntax.** `builtins`' `connectives` list (§5.5) makes `"then"` an unconditional clause separator at the `Lexer.clauses` stage. A rule pattern like `"if :target exists then :body"` can never match, because clause-splitting tears it into two clauses before Tier1/Tier2 ever see the full span — the pattern's literal `"then"` and the lexer's connective `"then"` collide. Considered two fixes: (a) scope connective-splitting to skip a span a control-flow tool's pattern would claim, or (b) reserve connectives as a no-go vocabulary for rule literal syntax and enforce it. Went with (b) — (a) would make `Lexer.clauses` (pure tokenizing/segmentation) reach into `Tier1`'s job (pattern matching), and makes "is this connective a real separator here?" implicit and input-dependent, the opposite of this project's deterministic/auditable goal. **Implemented** as `Ruleset.connective_collision(pattern, connectives)`, checked in two places: `RulesModule#tool` raises `ArgumentError` at ruleset-build time if an author-declared rule's literal words contain a connective sequence (e.g. `"archive :target then :body"` is rejected, forcing `"do"` instead — as `conditional` already does); the `teach` tool's action runs the same check at runtime and refuses the trigger with a user-facing message rather than silently learning a rule that can never fire. Loop syntax (`"for each :item in :list do :body"` or similar) should keep following the same `"do"`-joiner convention. Suggested next slice: generalize from a single flat `:body` action to a list of actions (still no `else`, no loops), then add the predicate struct, then loops — validating one extension at a time. +### 6.6 Command passthrough (`execute` tool) + +A fourth, last-resort resolution step after Tier 1/2 both miss: if a clause's first non-punctuation token is a recognized `$PATH` executable (Lexer's `:command` token, §5.1), it runs as a literal shell command instead of failing to resolve. **Implemented** as `CommandTool.lookup`, tried from `Utterance.resolve` only after `Tier1.lookup`/`Tier2.lookup` both miss, so a command name that also happens to be a declared verb (e.g. a tool literally named `archive`) still resolves through the normal rule first. + +This doesn't fit the `ToolBuilder`/Tier1/Tier2 pattern-matching machinery — there's no fixed trigger phrase to index, since the "pattern" is just "starts with any known executable" and the rest of the clause is arbitrary argv. `CommandTool::TOOL` is therefore a bare `Ruleset::Tool` built by hand (empty `rules`, so it never enters `Tier1.rules`/`Tier2`'s vocabulary) with its own `execute` action. + +**Security:** the action runs `Open3.capture3(*argv.map(&:text))` — passing argv as separate Ruby arguments, not one interpolated shell string — so Ruby/`Open3` execs the binary directly with no shell in between. Shell metacharacters in a literal token's text (`;`, `|`, `` ` ``, `$(...)`) are inert; there is no command-injection surface regardless of what a slot's value contains. The tradeoff: no glob expansion or `$VAR` substitution either, since there's no shell to do it — literals are passed through exactly as tokenized. + +Permission is always `:ask`, never `:yolo` — this is the least-constrained action in the system (arbitrary argv, no fixed semantics to reason about ahead of time), so per §10's destructive-action policy it must always confirm. `Executor.run`'s confirmation prompt was generalized to show the resolved args (not just the tool name) specifically so this confirmation isn't blind — e.g. `run execute git status? [y/N]` rather than just `run execute?`. + --- ## 7. Node Schemas (JSON Schema, abbreviated) diff --git a/nlshell.rb b/nlshell.rb index 8d8283b..cbb0e25 100755 --- a/nlshell.rb +++ b/nlshell.rb @@ -3,6 +3,7 @@ # frozen_string_literal: true require "set" require "strscan" +require "open3" # Scan the environment for possible shell commands ShellCommands = Set.new @@ -54,6 +55,10 @@ module Lexer # Sentence punctuation that greedy patterns may swallow ("rm ~/old.log.") TRAILING = /[.,;:!?]+\z/ + # Look like filenames but are ordinary words - their final period is part + # of the abbreviation, not a sentence end. + ABBREVIATIONS = %w[e.g i.e].freeze + # Ends a sentence. ";" also separates commands. BOUNDARIES = %w[. ? ! ;].freeze @@ -74,14 +79,19 @@ module Lexer if kind == :string text = text[1..-2] # drop quotes, keep content - elsif kind == :file and ShellCommands.include?(text) - return Token.new(:command, text, kind) else tail = text[TRAILING].to_s unless tail.empty? sc.pos -= tail.bytesize # hand sentence punctuation back to the scanner text = text.delete_suffix(tail) end + + return Token.new(:command, text, kind) if kind == :file && ShellCommands.include?(text) + + if kind == :file && ABBREVIATIONS.include?(text.downcase) + dot = sc.skip(/\./) ? "." : "" # its final period belongs to it, not a sentence end + return Token.new(:word, "#{text}#{dot}") + end end return Token.new(:literal, text, kind) @@ -453,16 +463,28 @@ module Executor # Confirms first for an :ask tool, same policy a top-level clause gets - the # one gate between "resolved" and "actually runs", so a conditional's body - # can't bypass permission checking. + # can't bypass permission checking. Shows the resolved args, not just the + # tool name, so a user never confirms blind - essential for `execute`, + # where the "args" are the literal command about to run. def self.run(node) tool = node[:tool] if tool.permission == :ask - print " run #{tool.name}? [y/N] " + print " run #{tool.name} #{describe(node[:args])}? [y/N] " return false unless gets&.chomp&.downcase == "y" end call(tool, node[:args]) end + + def self.describe(args) + args.map do |_, v| + case v + when Lexer::Token then v.text + when Array then v.map(&:text).join(" ") + else v.to_s + end + end.join(" ") + end end @@ -501,14 +523,17 @@ module Utterance Result.new(raw, clauses.map { |tokens| resolve(tokens) }) end - # Tries Tier 1 first (strict positional match), falling back to Tier 2 - # (fuzzy bag-of-words) only on a Tier 1 miss - tags which tier resolved it - # so the REPL can report that, or nil/nil if neither did. + # Tries Tier 1 first (strict positional match), then Tier 2 (fuzzy + # bag-of-words), then - last resort - raw shell command passthrough, so a + # recognized $PATH executable still runs even with no matching rule. Tags + # which tier resolved it so the REPL can report that, or nil/nil if none did. def self.resolve(tokens) if (node = Tier1.lookup(tokens)) Clause.new(tokens, node, :tier1) elsif (node = Tier2.lookup(tokens)) Clause.new(tokens, node, :tier2) + elsif (node = CommandTool.lookup(tokens)) + Clause.new(tokens, node, :command) else Clause.new(tokens, nil, nil) end @@ -526,6 +551,17 @@ module Ruleset pattern.split.each_with_object(Set.new) { |w, s| s << w.delete_prefix(":").to_sym if w.start_with?(":") } end + # A rule can't use a connective word/phrase as literal syntax: Lexer.clauses + # (SS5.5) splits on it before the rule ever sees the full span (e.g. "if + # :target exists then :body" is torn in two before Tier1 gets a look). + # Returns the colliding connective sequence, or nil. Shared by author-time + # rules (RulesModule#tool) and runtime-taught ones (the "teach" tool) so + # neither path can register a pattern clause-splitting would break. + def self.connective_collision(pattern, connectives) + words = pattern.split + connectives.find { |seq| words.each_cons(seq.length).any? { |window| window == seq } } + end + # Registers pattern -> tool in rules, after checking pattern's :slot names # match reference's. The one mechanism behind both author-declared verb # synonyms (ToolBuilder#verb_synonyms) and user-taught ones (the "teach" @@ -635,6 +671,7 @@ module Ruleset class RulesModule def initialize @rules = {} + @connectives = [] end def types(hash) @@ -648,7 +685,14 @@ module Ruleset def tool(name, &block) builder = ToolBuilder.new(name) builder.instance_eval(&block) - register(builder.build) + built = builder.build + built.rules.each do |pattern| + collision = Ruleset.connective_collision(pattern, @connectives) + next unless collision + + raise ArgumentError, "tool #{name.inspect}: rule #{pattern.inspect} uses connective #{collision.join(' ').inspect} as literal syntax - clause-splitting (SS5.5) would break it before the rule ever matches; pick a different joiner word" + end + register(built) end def literal_patterns = @types @@ -680,6 +724,31 @@ module Ruleset end end +# Last-resort resolution (used by Utterance.resolve after Tier1/2 both miss): +# a clause whose first non-punct token is a recognized $PATH executable +# (Lexer's :command token) runs as a literal shell command. Matched by token +# type, not a fixed trigger phrase, so it never goes through ToolBuilder/ +# Tier1/Tier2's pattern machinery - there's no vocabulary to index, since the +# "pattern" is just "starts with any known executable". +module CommandTool + # Open3.capture3 with separate argv elements execs the binary directly - + # no shell is ever invoked, so there's no shell-metacharacter/command- + # injection surface regardless of what a slot's literal text contains. + TOOL = Ruleset::Tool.new("execute", [], lambda { |argv:| + out, err, status = Open3.capture3(*argv.map(&:text)) + print out + warn err unless err.empty? + puts " exit #{status.exitstatus}" unless status.success? + }, :ask, Set.new) + + def self.lookup(clause) + argv = clause.reject(&:punct?) + return nil unless argv.first&.command? + + { type: "action", name: TOOL.name, args: { argv: argv }, tool: TOOL } + end +end + # Create default ruleset builtins = Ruleset.create("builtins") do types( @@ -775,6 +844,11 @@ builtins = Ruleset.create("builtins") do next end + if (collision = Ruleset.connective_collision(trigger, Lexer.connectives)) + puts " #{trigger.inspect} uses #{collision.join(' ').inspect} as literal words, but that's a clause-splitting connective - pick a different phrasing" + next + end + # Same slot-check-and-register mechanism as author-declared verb # synonyms (Ruleset::ToolBuilder#verb_synonyms). unless Ruleset.register_rule(Tier1.rules, trigger, tool, reference: target) diff --git a/test/nlshell_test.rb b/test/nlshell_test.rb index 9f0fef0..0b1e499 100644 --- a/test/nlshell_test.rb +++ b/test/nlshell_test.rb @@ -29,6 +29,14 @@ class LexerTest < Minitest::Test assert_equal 2, sentences.length end + # Regression: "e.g."/"i.e." must not be read as a file literal whose + # trailing period then ends the sentence early. + def test_abbreviation_period_does_not_end_a_sentence + tokens = Lexer.tokenize("keep text files, e.g. notes.txt, archived.") + assert_equal 1, Lexer.sentences(tokens).length + assert_equal :word, tokens.find { |t| t.text == "e.g." }.type + end + def test_clauses_split_on_connective tokens = Lexer.downcase_words(Lexer.tokenize("delete old.log then archive new.log")) clauses = Lexer.clauses(tokens) @@ -85,6 +93,44 @@ class UtteranceTest < Minitest::Test assert_nil clause.node assert_nil clause.tier end + + # Last-resort fallback: no rule matches "ls", but it's a recognized $PATH + # executable, so it resolves via command passthrough instead of failing. + def test_resolves_a_recognized_executable_via_command_passthrough + clause = Utterance.build("ls").clauses.first + assert_equal :command, clause.tier + assert_equal "execute", clause.node[:name] + end +end + +class CommandToolTest < Minitest::Test + def test_lookup_matches_a_clause_starting_with_a_command_token + tokens = Lexer.downcase_words(Lexer.tokenize("ls -la")) + node = CommandTool.lookup(tokens) + refute_nil node + assert_equal "execute", node[:name] + assert_equal %w[ls -la], node[:args][:argv].map(&:text) + end + + def test_lookup_misses_on_an_ordinary_word + tokens = Lexer.downcase_words(Lexer.tokenize("bork the garble")) + assert_nil CommandTool.lookup(tokens) + end + + def test_lookup_drops_trailing_sentence_punctuation + tokens = Lexer.downcase_words(Lexer.tokenize("ls.")) + node = CommandTool.lookup(tokens) + assert_equal ["ls"], node[:args][:argv].map(&:text) + end + + # The whole point of argv-form Open3 (no shell string) is that shell + # metacharacters in a literal's text can't be interpreted - this is the + # property that makes passthrough safe to add at all. + def test_execute_does_not_invoke_a_shell + argv = [Lexer::Token.new(:command, "echo"), Lexer::Token.new(:literal, "a; echo pwned", :string)] + out, = capture_io { CommandTool::TOOL.action.call(argv: argv) } + assert_equal "a; echo pwned\n", out + end end class Tier1Test < Minitest::Test @@ -178,6 +224,19 @@ class RulesetTest < Minitest::Test assert_match(/don't match/, error.message) end + # A connective word used as literal rule syntax would get torn out by + # Lexer.clauses before Tier1 ever sees the full pattern - must fail loudly + # at build time instead of silently never matching. + def test_tool_rejects_rule_using_a_connective_as_literal_syntax + error = assert_raises(ArgumentError) do + Ruleset.create("t") do + connectives [["then"]] + tool("bad") { rule "do :x then :y"; action { |x:, y:| } } + end + end + assert_match(/connective "then"/, error.message) + end + def test_tool_rejects_rule_starting_with_slot assert_raises(ArgumentError) do Ruleset.create("t") do @@ -229,6 +288,14 @@ class SharedRegistrationTest < Minitest::Test refute Ruleset.register_rule(rules, "wipe :thing", tool, reference: "delete :target") assert_nil rules["wipe :thing"] end + + def test_connective_collision_finds_a_literal_match + assert_equal ["then"], Ruleset.connective_collision("archive :target then :body", [["then"], ["next"]]) + end + + def test_connective_collision_ignores_a_slot_name + assert_nil Ruleset.connective_collision("if :target exists do :body", [["then"], ["next"]]) + end end class ExecutorTest < Minitest::Test @@ -289,11 +356,27 @@ class ReplIntegrationTest < Minitest::Test assert_match(/remembered: "wipe" means "delete"/, out) end + # A taught trigger that uses a connective word as literal syntax would be + # torn apart by clause-splitting before it could ever match - reject it at + # teach time instead of silently learning a rule that never fires. + def test_teach_rejects_a_trigger_that_collides_with_a_connective + out = run_repl(%(teach "zap :target then" means "delete :target"\ny\n)) + assert_match(/clause-splitting connective/, out) + refute_match(/learned:/, out) + end + def test_declining_confirmation_skips_execution out = run_repl(%(delete old.log\nn\n)) refute_match(/Deleting old\.log/, out) end + # The confirm prompt must show the literal command, not just the tool name + # - the user can't confirm blind on the riskiest action in the system. + def test_command_passthrough_prompts_with_the_literal_command + out = run_repl("ls\nn\n") + assert_match(/run execute ls\? \[y\/N\]/, out) + end + def test_unresolvable_input_reports_couldnt_tell out = run_repl(%(bork the garble\n)) assert_match(/couldn't tell what kind of request this is/, out)