Merge pull request #1101 from cantino/shell_command_agent_enhancement

ShellCommandAgent enhancement (stdin, argv, dry-run, no chdir)

Akinori MUSHA 9 years ago
parent
commit
ea98bf93f5
2 changed files with 131 additions and 33 deletions
  1. 64 24
      app/models/agents/shell_command_agent.rb
  2. 67 9
      spec/models/agents/shell_command_agent_spec.rb

+ 64 - 24
app/models/agents/shell_command_agent.rb

@@ -1,9 +1,9 @@
1
-require 'open3'
2
-
3 1
 module Agents
4 2
   class ShellCommandAgent < Agent
5 3
     default_schedule "never"
6 4
 
5
+    can_dry_run!
6
+
7 7
     def self.should_run?
8 8
       ENV['ENABLE_INSECURE_AGENTS'] == "true"
9 9
     end
@@ -11,7 +11,7 @@ module Agents
11 11
     description <<-MD
12 12
       The Shell Command Agent will execute commands on your local system, returning the output.
13 13
 
14
-      `command` specifies the command to be executed, and `path` will tell ShellCommandAgent in what directory to run this command.
14
+      `command` specifies the command (either a shell command line string or an array of command line arguments) to be executed, and `path` will tell ShellCommandAgent in what directory to run this command.  The content of `stdin` will be fed to the command via the standard input.
15 15
 
16 16
       `expected_update_period_in_days` is used to determine if the Agent is working.
17 17
 
@@ -20,6 +20,10 @@ module Agents
20 20
 
21 21
       The resulting event will contain the `command` which was executed, the `path` it was executed under, the `exit_status` of the command, the `errors`, and the actual `output`. ShellCommandAgent will not log an error if the result implies that something went wrong.
22 22
 
23
+      If `suppress_on_failure` is set to true, no event is emitted when `exit_status` is not zero.
24
+
25
+      If `suppress_on_empty_output` is set to true, no event is emitted when `output` is empty.
26
+
23 27
       *Warning*: This type of Agent runs arbitrary commands on your system, #{Agents::ShellCommandAgent.should_run? ? "but is **currently enabled**" : "and is **currently disabled**"}.
24 28
       Only enable this Agent if you trust everyone using your Huginn installation.
25 29
       You can enable this Agent in your .env file by setting `ENABLE_INSECURE_AGENTS` to `true`.
@@ -31,7 +35,7 @@ module Agents
31 35
         {
32 36
           "command": "pwd",
33 37
           "path": "/home/Huginn",
34
-          "exit_status": "0",
38
+          "exit_status": 0,
35 39
           "errors": "",
36 40
           "output": "/home/Huginn"
37 41
         }
@@ -41,6 +45,8 @@ module Agents
41 45
       {
42 46
           'path' => "/",
43 47
           'command' => "pwd",
48
+          'suppress_on_failure' => false,
49
+          'suppress_on_empty_output' => false,
44 50
           'expected_update_period_in_days' => 1
45 51
       }
46 52
     end
@@ -50,6 +56,16 @@ module Agents
50 56
         errors.add(:base, "The path, command, and expected_update_period_in_days fields are all required.")
51 57
       end
52 58
 
59
+      case options['stdin']
60
+      when String, nil
61
+      else
62
+        errors.add(:base, "stdin must be a string.")
63
+      end
64
+
65
+      unless Array(options['command']).all? { |o| o.is_a?(String) }
66
+        errors.add(:base, "command must be a shell command line string or an array of command line arguments.")
67
+      end
68
+
53 69
       unless File.directory?(options['path'])
54 70
         errors.add(:base, "#{options['path']} is not a real directory.")
55 71
       end
@@ -75,38 +91,62 @@ module Agents
75 91
       if Agents::ShellCommandAgent.should_run?
76 92
         command = opts['command']
77 93
         path = opts['path']
94
+        stdin = opts['stdin']
95
+
96
+        result, errors, exit_status = run_command(path, command, stdin)
78 97
 
79
-        result, errors, exit_status = run_command(path, command)
98
+        payload = {
99
+          'command' => command,
100
+          'path' => path,
101
+          'exit_status' => exit_status,
102
+          'errors' => errors,
103
+          'output' => result,
104
+        }
80 105
 
81
-        vals = {"command" => command, "path" => path, "exit_status" => exit_status, "errors" => errors, "output" => result}
82
-        created_event = create_event :payload => vals
106
+        unless suppress_event?(payload)
107
+          created_event = create_event payload: payload
108
+        end
83 109
 
84
-        log("Ran '#{command}' under '#{path}'", :outbound_event => created_event, :inbound_event => event)
110
+        log("Ran '#{command}' under '#{path}'", outbound_event: created_event, inbound_event: event)
85 111
       else
86 112
         log("Unable to run because insecure agents are not enabled.  Edit ENABLE_INSECURE_AGENTS in the Huginn .env configuration.")
87 113
       end
88 114
     end
89 115
 
90
-    def run_command(path, command)
91
-      result = nil
92
-      errors = nil
93
-      exit_status = nil
94
-
95
-      Dir.chdir(path){
96
-        begin
97
-          stdin, stdout, stderr, wait_thr = Open3.popen3(command)
98
-          exit_status = wait_thr.value.to_i
99
-          result = stdout.gets(nil)
100
-          errors = stderr.gets(nil)
101
-        rescue Exception => e
102
-          errors = e.to_s
116
+    def run_command(path, command, stdin)
117
+      begin
118
+        rout, wout = IO.pipe
119
+        rerr, werr = IO.pipe
120
+        rin,  win = IO.pipe
121
+
122
+        pid = spawn(*command, chdir: path, out: wout, err: werr, in: rin)
123
+
124
+        wout.close
125
+        werr.close
126
+        rin.close
127
+
128
+        if stdin
129
+          win.write stdin
130
+          win.close
103 131
         end
104
-      }
105 132
 
106
-      result = result.to_s.strip
107
-      errors = errors.to_s.strip
133
+        (result = rout.read).strip!
134
+        (errors = rerr.read).strip!
135
+
136
+        _, status = Process.wait2(pid)
137
+        exit_status = status.exitstatus
138
+      rescue => e
139
+        errors = e.to_s
140
+        result = ''.freeze
141
+        exit_status = nil
142
+      end
108 143
 
109 144
       [result, errors, exit_status]
110 145
     end
146
+
147
+    def suppress_event?(payload)
148
+      (boolify(interpolated['suppress_on_failure']) && payload['exit_status'].nonzero?) ||
149
+        (boolify(interpolated['suppress_on_empty_output']) && payload['output'].empty?)
150
+    end
111 151
   end
112 152
 end

+ 67 - 9
spec/models/agents/shell_command_agent_spec.rb

@@ -5,19 +5,31 @@ describe Agents::ShellCommandAgent do
5 5
     @valid_path = Dir.pwd
6 6
 
7 7
     @valid_params = {
8
-        :path  => @valid_path,
9
-        :command  => "pwd",
10
-        :expected_update_period_in_days => "1",
11
-      }
8
+      path: @valid_path,
9
+      command: 'pwd',
10
+      expected_update_period_in_days: '1',
11
+    }
12
+
13
+    @valid_params2 = {
14
+      path: @valid_path,
15
+      command: [RbConfig.ruby, '-e', 'puts "hello, #{STDIN.eof? ? "world" : STDIN.read.strip}."; STDERR.puts "warning!"'],
16
+      stdin: "{{name}}",
17
+      expected_update_period_in_days: '1',
18
+    }
12 19
 
13
-    @checker = Agents::ShellCommandAgent.new(:name => "somename", :options => @valid_params)
20
+    @checker = Agents::ShellCommandAgent.new(name: 'somename', options: @valid_params)
14 21
     @checker.user = users(:jane)
15 22
     @checker.save!
16 23
 
24
+    @checker2 = Agents::ShellCommandAgent.new(name: 'somename2', options: @valid_params2)
25
+    @checker2.user = users(:jane)
26
+    @checker2.save!
27
+
17 28
     @event = Event.new
18 29
     @event.agent = agents(:jane_weather_agent)
19 30
     @event.payload = {
20
-      :cmd => "ls"
31
+      'name' => 'Huginn',
32
+      'cmd' => 'ls',
21 33
     }
22 34
     @event.save!
23 35
 
@@ -27,6 +39,7 @@ describe Agents::ShellCommandAgent do
27 39
   describe "validation" do
28 40
     before do
29 41
       expect(@checker).to be_valid
42
+      expect(@checker2).to be_valid
30 43
     end
31 44
 
32 45
     it "should validate presence of necessary fields" do
@@ -47,7 +60,7 @@ describe Agents::ShellCommandAgent do
47 60
 
48 61
   describe "#working?" do
49 62
     it "generating events as scheduled" do
50
-      stub(@checker).run_command(@valid_path, 'pwd') { ["fake pwd output", "", 0] }
63
+      stub(@checker).run_command(@valid_path, 'pwd', nil) { ["fake pwd output", "", 0] }
51 64
 
52 65
       expect(@checker).not_to be_working
53 66
       @checker.check
@@ -60,7 +73,9 @@ describe Agents::ShellCommandAgent do
60 73
 
61 74
   describe "#check" do
62 75
     before do
63
-      stub(@checker).run_command(@valid_path, 'pwd') { ["fake pwd output", "", 0] }
76
+      stub(@checker).run_command(@valid_path, 'pwd', nil) { ["fake pwd output", "", 0] }
77
+      stub(@checker).run_command(@valid_path, 'empty_output', nil) { ["", "", 0] }
78
+      stub(@checker).run_command(@valid_path, 'failure', nil) { ["failed", "error message", 1] }
64 79
     end
65 80
 
66 81
     it "should create an event when checking" do
@@ -70,6 +85,42 @@ describe Agents::ShellCommandAgent do
70 85
       expect(Event.last.payload[:output]).to eq("fake pwd output")
71 86
     end
72 87
 
88
+    it "should create an event when checking (unstubbed)" do
89
+      expect { @checker2.check }.to change { Event.count }.by(1)
90
+      expect(Event.last.payload[:path]).to eq(@valid_path)
91
+      expect(Event.last.payload[:command]).to eq([RbConfig.ruby, '-e', 'puts "hello, #{STDIN.eof? ? "world" : STDIN.read.strip}."; STDERR.puts "warning!"'])
92
+      expect(Event.last.payload[:output]).to eq('hello, world.')
93
+      expect(Event.last.payload[:errors]).to eq('warning!')
94
+    end
95
+
96
+    describe "with suppress_on_empty_output" do
97
+      it "should suppress events on empty output" do
98
+        @checker.options[:suppress_on_empty_output] = true
99
+        @checker.options[:command] = 'empty_output'
100
+        expect { @checker.check }.not_to change { Event.count }
101
+      end
102
+
103
+      it "should not suppress events on non-empty output" do
104
+        @checker.options[:suppress_on_empty_output] = true
105
+        @checker.options[:command] = 'failure'
106
+        expect { @checker.check }.to change { Event.count }.by(1)
107
+      end
108
+    end
109
+
110
+    describe "with suppress_on_failure" do
111
+      it "should suppress events on failure" do
112
+        @checker.options[:suppress_on_failure] = true
113
+        @checker.options[:command] = 'failure'
114
+        expect { @checker.check }.not_to change { Event.count }
115
+      end
116
+
117
+      it "should not suppress events on success" do
118
+        @checker.options[:suppress_on_failure] = true
119
+        @checker.options[:command] = 'empty_output'
120
+        expect { @checker.check }.to change { Event.count }.by(1)
121
+      end
122
+    end
123
+
73 124
     it "does not run when should_run? is false" do
74 125
       stub(Agents::ShellCommandAgent).should_run? { false }
75 126
       expect { @checker.check }.not_to change { Event.count }
@@ -78,7 +129,7 @@ describe Agents::ShellCommandAgent do
78 129
 
79 130
   describe "#receive" do
80 131
     before do
81
-      stub(@checker).run_command(@valid_path, @event.payload[:cmd]) { ["fake ls output", "", 0] }
132
+      stub(@checker).run_command(@valid_path, @event.payload[:cmd], nil) { ["fake ls output", "", 0] }
82 133
     end
83 134
 
84 135
     it "creates events" do
@@ -89,6 +140,13 @@ describe Agents::ShellCommandAgent do
89 140
       expect(Event.last.payload[:output]).to eq("fake ls output")
90 141
     end
91 142
 
143
+    it "creates events (unstubbed)" do
144
+      @checker2.receive([@event])
145
+      expect(Event.last.payload[:path]).to eq(@valid_path)
146
+      expect(Event.last.payload[:output]).to eq('hello, Huginn.')
147
+      expect(Event.last.payload[:errors]).to eq('warning!')
148
+    end
149
+
92 150
     it "does not run when should_run? is false" do
93 151
       stub(Agents::ShellCommandAgent).should_run? { false }
94 152